From 5fe23383832f70eb119880347ac693c8ff035672 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sat, 29 Aug 2026 23:48:15 -0400 Subject: [PATCH 1/9] fix(coverage): narrow parse-error ranges with the preprocessed parse (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/cli/cli.c reported an error range of 1-13047 — the whole file. The file indexed fine; the report was wrong. Three #ifndef _WIN32 blocks split a brace (two `if` headers, one closing brace), so the raw tree-sitter parse cannot resync at file scope, the root node becomes ERROR, and cbm.c takes its whole-file branch. The pipeline already parses these files a second time after preprocessing, and that parse is clean. The report just never consulted it. Build one byte per original line from the preprocessed pass, then cut each raw error range down to the runs of lines the second parse could not vouch for. Three rules, all found by running it and all load-bearing: - An expanded line only vouches for its original line when it HAS TEXT. The preprocessor emits a blank line where it dropped a branch; treating that blank as proof suppressed every C range in the suite. - Preprocessor directive lines (with backslash continuations) never count as missing code — the preprocessor consumes them, so the second parse can never vouch for one. Without this every #include block reported as a miss. Known cost: a #define the raw parse really dropped no longer shows up on its own. - A TOP-LEVEL macro invocation line never counts as vouched-for even when the expanded line parses clean. The macro can expand to a whole definition that the recovery walker deliberately refuses to adopt (#949), so a clean second parse there proves nothing. An in-body invocation is the benign #1071 case and is left to the existing macro subtraction. The order of the three coverage steps is now settled by where each one's evidence lives: recovery subtraction -> before the refinement; its evidence is a whole definition that STARTS inside the range, so it must be asked while the range still matches the construct the refinement -> middle #1071 macro rule -> after the refinement; its evidence is per-line, so a narrow range points at the call itself Measured on this repo: src/cli/cli.c goes from one whole-file range to 64 ranges over ~9.8% of the file, tests/test_cli.c from 48.6% to ~2.9%, src/cli/activation_transaction.c from 38% to 7.5%. What survives is honest — the biggest remaining ranges in cli.c are genuinely discarded #ifdef _WIN32 and #ifdef CBM_CLI_ENABLE_TEST_API blocks, absent from the graph on this platform. Both percentages above are floors, not measurements: cli.c and test_cli.c now land on exactly 64 ranges, which is CBM_MAX_ERROR_REGIONS. That cap drops regions with no signal, and a follow-up raises it and adds a truncation marker. Five tests, all red before the change: the range narrows to the dropped branch; lines the preprocessor explained are excluded; a range never starts or ends on a directive; real garbage beside a split brace stays flagged; a clean file stays unflagged. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- internal/cbm/cbm.c | 272 +++++++++++++++++++++++++++++++++++- tests/test_parse_coverage.c | 139 ++++++++++++++++++ 2 files changed, 410 insertions(+), 1 deletion(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index da6e720a2..5a20dd025 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -862,6 +862,130 @@ static void cbm_collect_error_regions(TSNode n, cbm_error_regions_t *acc, const } } +/* ── Phase 2 line map: what the preprocessed parse already explained ─────── + * + * The raw parse is preprocessor-blind. When an #ifdef splits a brace it sees + * both branches at once, the braces do not balance, and the ERROR node + * swallows the whole construct — at file scope it swallows the whole FILE. + * The second parse, on preprocessed source, does not have that problem: the + * preprocessor already picked one branch, so that parse is clean. + * + * So we build one byte per ORIGINAL line and use it to cut the raw ranges + * down to the lines the second parse cannot vouch for. Lines in the branch + * the preprocessor threw away never appear in the second parse at all, so + * they stay flagged — which is right, because they really are missing from + * the graph. + * + * CBM_LINE_PP_PARSED — the preprocessed parse covered this original line and + * found no error on it. Nothing here was dropped. + * CBM_LINE_NO_CODE — the line is empty, is only a comment, or is a + * preprocessor directive. A reported range must never + * begin or end on one. + * + * Directives are in this set because the preprocessor + * CONSUMES them: no directive line ever survives into + * the expanded text, so the second parse can never + * vouch for one, and treating that silence as a miss + * would flag every #include block in the file. The + * known cost is a #define that the raw parse really did + * drop: it no longer shows up on its own. That trade is + * deliberate — it removes far more noise than signal. */ +enum { CBM_LINE_PP_PARSED = 1u, CBM_LINE_NO_CODE = 2u }; + +/* True when the line's first non-blank character starts a preprocessor + * directive. */ +static bool cbm_is_directive_line(const char *line, int len) { + int i = 0; + while (i < len && (line[i] == ' ' || line[i] == '\t')) { + i++; + } + return i < len && line[i] == '#'; +} + +/* True when the line ends with a backslash, so the directive carries on to + * the next line. */ +static bool cbm_line_continues(const char *line, int len) { + int end = len; + while (end > 0 && (line[end - 1] == ' ' || line[end - 1] == '\t' || line[end - 1] == '\r')) { + end--; + } + return end > 0 && line[end - 1] == '\\'; +} + +/* Set CBM_LINE_NO_CODE on every line of `src` that holds no construct. + * One pass over the file. Carries block-comment state across lines so a line + * in the middle of a comment counts as no-code too. */ +static void cbm_mark_no_code_lines(const char *src, int src_len, uint8_t *map, + uint32_t line_count) { + bool in_block = false; + bool in_directive = false; + uint32_t line = 1; + int i = 0; + while (i <= src_len && line <= line_count) { + int end = i; + while (end < src_len && src[end] != '\n') { + end++; + } + bool has_code = false; + bool line_starts_in_block = in_block; + for (int j = i; j < end; j++) { + if (in_block) { + if (src[j] == '*' && j + 1 < end && src[j + 1] == '/') { + in_block = false; + j++; + } + continue; + } + if (src[j] == '/' && j + 1 < end && src[j + 1] == '*') { + in_block = true; + j++; + continue; + } + if (src[j] == '/' && j + 1 < end && src[j + 1] == '/') { + break; /* rest of the line is a comment */ + } + if (src[j] != ' ' && src[j] != '\t' && src[j] != '\r') { + has_code = true; + } + } + bool directive = + !line_starts_in_block && (in_directive || cbm_is_directive_line(src + i, end - i)); + if (!has_code || directive) { + map[line] |= CBM_LINE_NO_CODE; + } + in_directive = directive && cbm_line_continues(src + i, end - i); + line++; + i = end + 1; + } +} + +/* Paint CBM_LINE_PP_PARSED for every original line the preprocessed parse + * covered without an error on it. + * + * Step 1 marks the EXPANDED rows that sit under an ERROR/MISSING node. + * Step 2 walks the expanded lines and, for each one that is unmarked, belongs + * to the file itself (not an included header) and maps back to a real + * original line, records that original line as parsed. */ +static void cbm_mark_pp_error_rows(TSNode n, uint8_t *rows, uint32_t row_count, const char *src, + int src_len) { + uint32_t k = ts_node_child_count(n); + for (uint32_t i = 0; i < k; i++) { + TSNode c = ts_node_child(n, i); + if (ts_node_is_missing(c) || strcmp(ts_node_type(c), "ERROR") == 0) { + if (cbm_is_eof_terminator_miss(c, src, src_len)) { + continue; /* absent final newline only — nothing was dropped */ + } + uint32_t s = ts_node_start_point(c).row + 1; + uint32_t e = ts_node_end_point(c).row + 1; + for (uint32_t r = s; r <= e && r <= row_count; r++) { + rows[r] = 1; + } + } else if (ts_node_has_error(c)) { + cbm_mark_pp_error_rows(c, rows, row_count, src, src_len); + } + } +} + /* Recovery subtraction (#963): tree-sitter error recovery plus the * ERROR-descending def walker often still extract constructs INSIDE a failed * region (verified: a function in an #ifdef-split ERROR region and even a @@ -1155,6 +1279,75 @@ static void cbm_subtract_macro_invocation_regions(cbm_error_regions_t *regs, regs->count = kept; } +/* Push [start, end] after trimming no-code lines off both ends. A run made + * only of directives, comments or blank lines disappears entirely — there was + * never a construct on it to lose. */ +static void cbm_push_trimmed_run(cbm_error_regions_t *out, uint32_t start, uint32_t end, + const uint8_t *map, uint32_t line_count) { + while (start <= end && start <= line_count && (map[start] & CBM_LINE_NO_CODE)) { + start++; + } + while (end >= start && end <= line_count && (map[end] & CBM_LINE_NO_CODE)) { + end--; + } + if (start > end || out->count >= CBM_MAX_ERROR_REGIONS) { + return; + } + out->starts[out->count] = start; + out->ends[out->count] = end; + out->count++; +} + +/* #949: a top-level macro invocation is the one place where a clean second + * parse proves nothing. The macro can expand to a whole definition, and the + * recovery walker deliberately refuses to adopt that definition because it is + * absent from the original span. So the expanded line parses fine while the + * construct really is missing from the graph, and the line must stay flagged. + * An invocation INSIDE a function body is the benign #1071 case and is left + * alone here — cbm_subtract_macro_invocation_regions handles it later. */ +static bool cbm_line_is_toplevel_macro_call(const char *src, int src_len, uint32_t line, + const CBMDefArray *defs) { + return cbm_span_is_macro_invocation(src, src_len, line, line, defs) && + !cbm_region_inside_callable(line, line, defs); +} + +/* Cut every raw region down to the lines the preprocessed parse could not + * vouch for. Each region becomes zero or more smaller ranges: one per run of + * consecutive lines that the second parse did not cover cleanly. + * + * This is what collapses a whole-file range on a file whose only real problem + * is an #ifdef splitting a brace. It deliberately does NOT clear the region + * outright — the branch the preprocessor discarded is genuinely absent from + * the graph and must stay flagged. */ +static void cbm_refine_regions_with_pp_lines(cbm_error_regions_t *regs, const uint8_t *map, + uint32_t line_count, const char *src, int src_len, + const CBMDefArray *defs) { + cbm_error_regions_t out = {{0}, {0}, 0}; + for (int i = 0; i < regs->count; i++) { + uint32_t run_start = 0; + uint32_t run_end = 0; + uint32_t end = regs->ends[i] < line_count ? regs->ends[i] : line_count; + for (uint32_t line = regs->starts[i]; line <= end; line++) { + if ((map[line] & CBM_LINE_PP_PARSED) && + !cbm_line_is_toplevel_macro_call(src, src_len, line, defs)) { + if (run_start != 0) { + cbm_push_trimmed_run(&out, run_start, run_end, map, line_count); + run_start = 0; + } + } else { + if (run_start == 0) { + run_start = line; + } + run_end = line; + } + } + if (run_start != 0) { + cbm_push_trimmed_run(&out, run_start, run_end, map, line_count); + } + } + *regs = out; +} + /* Serialize collected regions as "start-end,start-end,..." into the arena. */ static const char *cbm_error_ranges_str(CBMArena *a, const cbm_error_regions_t *regs) { if (regs->count <= 0) { @@ -1394,6 +1587,14 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua // metrics. Remember the boundary. int orig_calls_count = result->calls.count; + /* Phase 2 line map, built by the second (preprocessed) pass below and read + * by the parse-coverage block near the end of this function. Stays NULL + * for every language that has no second pass, which leaves the coverage + * signal exactly as it was. Arena-allocated so it outlives the + * preprocessed source and its tree. */ + uint8_t *pp_line_map = NULL; + uint32_t pp_line_map_lines = 0; + // Second pass: preprocess C/C++/CUDA and extract additional macro-hidden calls. // Defs keep original-source line numbers; only CALLS are extracted from expanded source. if (language == CBM_LANG_C || language == CBM_LANG_CPP || language == CBM_LANG_CUDA) { @@ -1519,6 +1720,60 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua } } + /* Build the original-line map before the expanded tree + * goes away. Skipped when the expanded parse is itself a + * total loss (root is ERROR), because then it vouches for + * nothing and there is no refinement to make. */ + if (strcmp(ts_node_type(pp_root), "ERROR") != 0) { + uint32_t orig_lines = 1; + for (int ci = 0; ci < source_len; ci++) { + if (source[ci] == '\n') { + orig_lines++; + } + } + uint8_t *map = + (uint8_t *)cbm_arena_alloc(a, (size_t)orig_lines + 2); + int exp_lines = preprocessed->expanded_line_count; + uint8_t *bad_rows = + exp_lines > 0 ? (uint8_t *)calloc((size_t)exp_lines + 2, 1) : NULL; + if (map && bad_rows) { + memset(map, 0, (size_t)orig_lines + 2); + cbm_mark_no_code_lines(source, source_len, map, orig_lines); + cbm_mark_pp_error_rows(pp_root, bad_rows, (uint32_t)exp_lines, + expanded, expanded_len); + /* Walk the expanded text once. An expanded line + * only vouches for its original line when it + * actually HAS content: the preprocessor emits a + * blank line where it dropped a branch, and a + * blank line proves nothing about the code that + * used to be there. */ + uint32_t eline = 1; + bool eline_has_text = false; + for (int ci = 0; ci <= expanded_len; ci++) { + if (ci < expanded_len && expanded[ci] != '\n') { + char ch = expanded[ci]; + if (ch != ' ' && ch != '\t' && ch != '\r') { + eline_has_text = true; + } + continue; + } + if (eline_has_text && (int)eline <= exp_lines && !bad_rows[eline] && + preprocessed->belongs_to_main_file[eline]) { + uint32_t orig = + preprocessed->original_line_by_expanded_line[eline]; + if (orig >= 1 && orig <= orig_lines) { + map[orig] |= CBM_LINE_PP_PARSED; + } + } + eline++; + eline_has_text = false; + } + pp_line_map = map; + pp_line_map_lines = orig_lines; + } + free(bad_rows); + } + ts_tree_delete(pp_tree); } } @@ -1640,9 +1895,24 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua } else { cbm_collect_error_regions(root, ®s, source, source_len); } + /* Recovery subtraction runs on the RAW ranges, before the Phase 2 + * refinement below. Its evidence is a whole definition that starts + * inside the range, so it has to be asked while the range still + * matches the construct. Ask it after the refinement and an #ifdef + * splitting a brace inside a recovered function looks unrecovered: + * the refinement keeps only the discarded branch, the function starts + * above it, and the evidence falls outside the range. */ cbm_subtract_recovered_regions(®s, &result->defs); + /* Phase 2: cut what is left down to the lines the preprocessed parse + * could not explain. */ + if (pp_line_map) { + cbm_refine_regions_with_pp_lines(®s, pp_line_map, pp_line_map_lines, source, + source_len, &result->defs); + } /* #1071: don't flag a benign function-like-macro call (defined in-file) - * that tree-sitter can't parse without the preprocessor. */ + * that tree-sitter can't parse without the preprocessor. Runs AFTER the + * refinement, because its evidence is per-line: a narrow range points at + * the call itself instead of the whole blob around it. */ cbm_subtract_macro_invocation_regions(®s, &result->defs, source, source_len); if (regs.count > 0) { result->parse_incomplete = true; diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index ba741531d..0c913837b 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -104,6 +104,29 @@ static const char *PY_CLEAN = "def ok():\n" "def ok2():\n" " return 2\n"; +/* #1610 fixtures follow. Refinement fixtures live here so they sit beside the + * split-brace fixture they build on. */ + +/* Same split-brace shape as C_IFDEF_SPLIT, plus real garbage further down. + * Guards against over-suppression: the preprocessor explains the guarded + * region but explains nothing about the garbage, so BOTH must stay flagged + * and they must be reported as two separate ranges, not one big one. */ +static const char *C_IFDEF_SPLIT_PLUS_GARBAGE = "#include \n" /* 1 */ + "\n" /* 2 */ + "void ok_before(void) { }\n" /* 3 */ + "\n" /* 4 */ + "#ifdef FEATURE_A\n" /* 5 */ + "static int guarded(int x) {\n" /* 6 */ + "#else\n" /* 7 */ + "static int guarded_alt(int x) {\n" /* 8 */ + "#endif\n" /* 9 */ + " return x + 1;\n" /* 10 */ + "}\n" /* 11 */ + "\n" /* 12 */ + "%%% ((( &&& ))) %%%\n" /* 13 */ + "\n" /* 14 */ + "void ok_after(void) { }\n"; /* 15 */ + /* Perl formats have a line-oriented body terminated by a lone dot. The * following named sub pins the important recovery boundary: a grammar must * both accept the format and resume normal declaration parsing afterwards. */ @@ -470,6 +493,117 @@ TEST(perl_malformed_source_remains_partial_issue1838) { PASS(); } +/* ── Phase 2: refine raw ranges with the preprocessed tree ────────────────── + * + * The raw parse sees both #ifdef branches at once, so its ERROR node covers + * the whole guarded construct (lines 5-11). The PREPROCESSED parse sees only + * the branch the preprocessor picked, and parses it clean. Every original + * line that shows up clean in that second parse is therefore accounted for, + * and reporting it as unparsed is false. + * + * What is left is the branch the preprocessor threw away — line 6 here. That + * one really is missing from the graph, so it stays flagged. Directive lines + * (#ifdef / #else / #endif) hold no construct, so a range never starts or + * ends on one. + */ + +/* Return 1 if the "a-b,c-d" range string covers 1-based `line`. */ +static int ranges_cover_line(const char *ranges, unsigned int line) { + const char *p = ranges; + while (p && *p) { + unsigned int s = 0, e = 0; + if (sscanf(p, "%u-%u", &s, &e) == 2 && line >= s && line <= e) { + return 1; + } + p = strchr(p, ','); + if (p) { + p++; + } + } + return 0; +} + +/* Total lines covered by every range in the string. */ +static unsigned int ranges_total_span(const char *ranges) { + const char *p = ranges; + unsigned int total = 0; + while (p && *p) { + unsigned int s = 0, e = 0; + if (sscanf(p, "%u-%u", &s, &e) == 2 && e >= s) { + total += e - s + 1; + } + p = strchr(p, ','); + if (p) { + p++; + } + } + return total; +} + +TEST(c_ifdef_split_range_narrows_to_dropped_branch) { + /* RED before the refinement: the raw range covers the whole 5-11 + * construct. GREEN after: only line 6, the branch the preprocessor did + * not pick, is still reported. */ + CBMFileResult *r = do_extract(C_IFDEF_SPLIT, CBM_LANG_C, "split.c"); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_NOT_NULL(r->error_ranges); + ASSERT_TRUE(ranges_cover_line(r->error_ranges, 6u)); /* dropped branch */ + ASSERT_LTE(ranges_total_span(r->error_ranges), 3u); /* was 7 lines */ + cbm_free_result(r); + PASS(); +} + +TEST(c_ifdef_split_range_excludes_lines_the_preprocessor_explained) { + /* Lines 10 and 11 are the shared body and closing brace. They parse + * clean once a branch is chosen, so pointing an agent at them is wrong. */ + CBMFileResult *r = do_extract(C_IFDEF_SPLIT, CBM_LANG_C, "split.c"); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(r->error_ranges); + ASSERT_FALSE(ranges_cover_line(r->error_ranges, 10u)); + ASSERT_FALSE(ranges_cover_line(r->error_ranges, 11u)); + ASSERT_FALSE(ranges_cover_line(r->error_ranges, 3u)); /* ok_before */ + cbm_free_result(r); + PASS(); +} + +TEST(c_ifdef_split_range_never_starts_on_a_directive) { + /* Lines 5, 7 and 9 are bare #ifdef / #else / #endif. No construct can + * live on them, so they must not appear in a range. */ + CBMFileResult *r = do_extract(C_IFDEF_SPLIT, CBM_LANG_C, "split.c"); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(r->error_ranges); + ASSERT_FALSE(ranges_cover_line(r->error_ranges, 5u)); + ASSERT_FALSE(ranges_cover_line(r->error_ranges, 7u)); + ASSERT_FALSE(ranges_cover_line(r->error_ranges, 9u)); + cbm_free_result(r); + PASS(); +} + +TEST(c_refinement_does_not_suppress_real_garbage) { + /* Anti-over-suppression. The preprocessor cannot explain line 13, so it + * stays flagged even though the guarded region above it narrowed. */ + CBMFileResult *r = do_extract(C_IFDEF_SPLIT_PLUS_GARBAGE, CBM_LANG_C, "both.c"); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_NOT_NULL(r->error_ranges); + ASSERT_TRUE(ranges_cover_line(r->error_ranges, 13u)); /* the garbage */ + ASSERT_FALSE(ranges_cover_line(r->error_ranges, 3u)); /* ok_before */ + cbm_free_result(r); + PASS(); +} + +TEST(c_clean_file_stays_unflagged_after_refinement) { + /* The refinement must never invent a range on a file that parses. */ + CBMFileResult *r = do_extract(C_CLEAN, CBM_LANG_C, "clean.c"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->parse_incomplete); + ASSERT_NULL(r->error_ranges); + cbm_free_result(r); + PASS(); +} + + SUITE(parse_coverage) { RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete); RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted); @@ -489,6 +623,11 @@ SUITE(parse_coverage) { RUN_TEST(missing_final_newline_not_flagged_across_grammars_issue1610); RUN_TEST(real_error_before_eof_still_flagged_without_final_newline_issue1610); RUN_TEST(width_bearing_error_at_eof_still_flagged_issue1610); + RUN_TEST(c_ifdef_split_range_narrows_to_dropped_branch); + RUN_TEST(c_ifdef_split_range_excludes_lines_the_preprocessor_explained); + RUN_TEST(c_ifdef_split_range_never_starts_on_a_directive); + RUN_TEST(c_refinement_does_not_suppress_real_garbage); + RUN_TEST(c_clean_file_stays_unflagged_after_refinement); RUN_TEST(perl_format_followed_by_named_sub_is_complete_issue1838); RUN_TEST(perl_malformed_source_remains_partial_issue1838); } From 23947912b538cdc29cab23cf0fc51c9fba371f64 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 00:32:39 -0400 Subject: [PATCH 2/9] fix(coverage): stop the report hiding what it dropped, and name whole-file failures (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two silent failures in the parse-coverage report, both made visible by the Phase 2 range refinement that came before this. ## The caps dropped ranges with no signal Two caps sat in series and both returned early without saying anything: CBM_MAX_ERROR_REGIONS = 64 internal/cbm/cbm.c COVERAGE_RANGE_MAX = 128 src/mcp/mcp.c Raising only the first would have moved the clip from 64 to 128, so both move to 256. This was live behaviour, not a theoretical limit: after Phase 2 split one whole-file range into many small ones, src/cli/cli.c and tests/test_cli.c both reported exactly 64 ranges — the cap binding, dead-on, twice. Every coverage figure measured before this change was a floor. With the cap at 256 the true numbers are cli.c 13.9% (not 9.8%) and test_cli.c 3.1%, and the longest list in the repo is 85 ranges. A raised cap is still a cap, so the report now says when it clipped: - cbm_error_regions_t gained a `dropped` counter, and cbm_collect_error_regions walks to the end instead of stopping at the cap, so the count is exact rather than a lower bound. That costs little — the walk never descends into an ERROR subtree. - cbm_error_ranges_str appends ",+" when N ranges were thrown away. - coverage_add_ranges reads that marker and sets "truncated": true, and also sets it when its own limit stops the loop. Before this the marker was invisible: the parser stopped at the '+' with no error and no leftover, so a clipped list arrived looking complete. - objectscript_export_append_error_ranges strips markers off both operands before joining two Studio Export parts and adds one back at the end. A marker left mid-string would make every reader stop there and silently lose every range after it. ## A whole-file range is not advice "Look at lines 1 to 13047" of a 13046-line file tells a reader nothing. Those files now carry their own kind rather than being described as partially covered. New `parse_unusable` field in CBMFileResult, set when one range covers 80% or more of the file. Its customers are non-C languages: the Phase 2 refinement that narrows a whole-file range using the preprocessed parse only runs for C, C++ and CUDA, so a Python, Java, Ruby or TypeScript file whose root node is ERROR still reports 1-N. Verified against real files in all four. The kind is `parse_unusable`, not `parse_failed`. index_coverage.kind already means one of two things — indexed-but-partial, or a skip phase saying the file was never indexed at all — and `parse_failed` reads as the second when it is the first. The store.c schema comment, which is the only written record of this vocabulary, now describes all three classes and says why. Two places would have mislabelled the new kind as "skipped", which is exactly that confusion: coverage_status fell through to its catch-all pass, and add_coverage_report fell into its else branch. A reader who finds a file under "skipped" believes it is absent from the graph, when it was indexed. Both now have explicit branches. index_status gained parse_unusable_count so a CI gate can read it without parsing anything else, get_code_snippet says "read the source directly" instead of naming useless ranges, and the three tool descriptions that listed two coverage kinds now list three. ## Tests Seven added. The cap test moved from 64 to 256; a new test asserts the marker carries a real drop count and that nothing follows it; an inverse test asserts an under-cap file carries no marker at all. For the new kind: a Python file whose root is ERROR is unusable, a file with a local parse failure stays partial, a clean file is neither, and — the one that matters most — the #ifdef-split C file that started this work is partial and never unusable. If that last one ever flips, the Phase 2 refinement has stopped working. Full suite: 7732 passed, 28 failed, 7 skipped. The 28 are pre-existing agent-client install/uninstall failures in the cli suite, identical in count and identity at clean HEAD. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- internal/cbm/cbm.c | 85 +++++++++++++--- internal/cbm/cbm.h | 15 +++ src/mcp/mcp.c | 167 ++++++++++++++++++++++++++++---- src/pipeline/pass_definitions.c | 68 ++++++++++++- src/pipeline/pass_parallel.c | 9 +- src/store/store.c | 22 ++++- tests/test_parse_coverage.c | 126 +++++++++++++++++++++++- 7 files changed, 447 insertions(+), 45 deletions(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index 5a20dd025..049d1cd1e 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -774,16 +774,24 @@ static bool cbm_source_nesting_exceeds(const char *source, int source_len, int c * nodes (does not descend into an error subtree — one range per failed region). * Bounded by CBM_MAX_ERROR_REGIONS so pathological input can't blow up the * output. The ranges mark where constructs were dropped; they are a detection - * aid, never a completeness proof. */ -#define CBM_MAX_ERROR_REGIONS 64 + * aid, never a completeness proof. + * + * `dropped` counts the ranges the cap threw away. It exists so a clipped list + * cannot read as a complete one: cbm_error_ranges_str turns a non-zero count + * into a trailing "+" marker. Phase 2 split one whole-file range into many + * small ones, which pushed real files straight into a cap that used to be + * unreachable, so the clip is live behaviour and not a theoretical limit. */ +#define CBM_MAX_ERROR_REGIONS 256 typedef struct { uint32_t starts[CBM_MAX_ERROR_REGIONS]; uint32_t ends[CBM_MAX_ERROR_REGIONS]; int count; + int dropped; } cbm_error_regions_t; static void cbm_error_regions_push(cbm_error_regions_t *acc, TSNode n) { if (acc->count >= CBM_MAX_ERROR_REGIONS) { + acc->dropped++; return; } acc->starts[acc->count] = ts_node_start_point(n).row + 1; @@ -843,13 +851,14 @@ static bool cbm_is_eof_terminator_miss(TSNode n, const char *source, int source_ return true; } +/* Walks to the end even after the cap is full, so `dropped` is the real number + * of ranges lost rather than a lower bound. This costs little: the walk never + * descends into an ERROR subtree — it records the top-most node and moves on — + * so it only visits the spine of nodes that contain an error, plus one level. */ static void cbm_collect_error_regions(TSNode n, cbm_error_regions_t *acc, const char *source, int source_len) { - if (acc->count >= CBM_MAX_ERROR_REGIONS) { - return; - } uint32_t k = ts_node_child_count(n); - for (uint32_t i = 0; i < k && acc->count < CBM_MAX_ERROR_REGIONS; i++) { + for (uint32_t i = 0; i < k; i++) { TSNode c = ts_node_child(n, i); if (ts_node_is_missing(c) || strcmp(ts_node_type(c), "ERROR") == 0) { if (cbm_is_eof_terminator_miss(c, source, source_len)) { @@ -1290,7 +1299,11 @@ static void cbm_push_trimmed_run(cbm_error_regions_t *out, uint32_t start, uint3 while (end >= start && end <= line_count && (map[end] & CBM_LINE_NO_CODE)) { end--; } - if (start > end || out->count >= CBM_MAX_ERROR_REGIONS) { + if (start > end) { + return; /* nothing but blank, comment or directive lines — no construct lost */ + } + if (out->count >= CBM_MAX_ERROR_REGIONS) { + out->dropped++; return; } out->starts[out->count] = start; @@ -1322,7 +1335,7 @@ static bool cbm_line_is_toplevel_macro_call(const char *src, int src_len, uint32 static void cbm_refine_regions_with_pp_lines(cbm_error_regions_t *regs, const uint8_t *map, uint32_t line_count, const char *src, int src_len, const CBMDefArray *defs) { - cbm_error_regions_t out = {{0}, {0}, 0}; + cbm_error_regions_t out = {{0}, {0}, 0, regs->dropped}; for (int i = 0; i < regs->count; i++) { uint32_t run_start = 0; uint32_t run_end = 0; @@ -1349,12 +1362,40 @@ static void cbm_refine_regions_with_pp_lines(cbm_error_regions_t *regs, const ui } /* Serialize collected regions as "start-end,start-end,..." into the arena. */ +/* Share of a file one range must cover before the range stops being advice and + * becomes noise. 80% is well clear of anything real: the widest single range in + * this repo covers 25.5% of its file, and the next widest 3.9%. */ +#define CBM_UNUSABLE_PCT 80 + +/* Number of 1-based lines in `src`. A file that does not end with a newline + * still has a last line, so the count is separators plus one. */ +static uint32_t cbm_count_lines(const char *src, int src_len) { + uint32_t n = 1; + for (int i = 0; i < src_len; i++) { + if (src[i] == '\n' && i + 1 < src_len) { + n++; + } + } + return n; +} + +/* Serialize collected regions as "start-end,start-end,...", with a trailing + * ",+" when the cap threw N ranges away. + * + * The marker must stay a SUFFIX and nothing else. Every reader stops at the + * first token that is not a range, so a marker in the middle of a string + * silently hides everything after it. objectscript_export_append_error_ranges + * strips markers before joining two parts for exactly that reason. + * + * N can be non-zero while the kept list is short, because the recovery and + * macro rules run after collection and remove ranges the cap never saw. That + * still reports honestly: the cap bound, so what was lost is unknown. */ static const char *cbm_error_ranges_str(CBMArena *a, const cbm_error_regions_t *regs) { - if (regs->count <= 0) { + if (regs->count <= 0 && regs->dropped <= 0) { return NULL; } enum { RANGE_MAX = 24 }; /* "4294967295-4294967295," */ - char *buf = (char *)cbm_arena_alloc(a, (size_t)regs->count * RANGE_MAX); + char *buf = (char *)cbm_arena_alloc(a, (size_t)(regs->count + 1) * RANGE_MAX); if (!buf) { return NULL; } @@ -1363,6 +1404,9 @@ static const char *cbm_error_ranges_str(CBMArena *a, const cbm_error_regions_t * off += (size_t)snprintf(buf + off, RANGE_MAX, "%s%u-%u", i ? "," : "", regs->starts[i], regs->ends[i]); } + if (regs->dropped > 0) { + snprintf(buf + off, RANGE_MAX, "%s+%d", off ? "," : "", regs->dropped); + } return buf; } @@ -1680,7 +1724,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua * the raw source line, and whose QN the raw pass did not * already extract. */ if (ts_node_has_error(root)) { - cbm_error_regions_t raw_regs = {{0}, {0}, 0}; + cbm_error_regions_t raw_regs = {{0}, {0}, 0, 0}; cbm_collect_error_regions(root, &raw_regs, source, source_len); if (raw_regs.count > 0) { int defs_before = result->defs.count; @@ -1889,7 +1933,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua * miss, and a fully recovered file is not flagged at all. Detection aid * only: the absence of this flag is NOT a completeness guarantee. */ if (ts_node_has_error(root)) { - cbm_error_regions_t regs = {{0}, {0}, 0}; + cbm_error_regions_t regs = {{0}, {0}, 0, 0}; if (strcmp(ts_node_type(root), "ERROR") == 0) { cbm_error_regions_push(®s, root); /* whole file unparseable */ } else { @@ -1914,10 +1958,25 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua * refinement, because its evidence is per-line: a narrow range points at * the call itself instead of the whole blob around it. */ cbm_subtract_macro_invocation_regions(®s, &result->defs, source, source_len); - if (regs.count > 0) { + /* A file whose kept list is empty but whose cap still bound is NOT clean: + * the ranges the cap threw away were never judged by the two rules + * above, so nothing proves they were recovered. Flag it. */ + if (regs.count > 0 || regs.dropped > 0) { result->parse_incomplete = true; result->error_region_count = regs.count; result->error_ranges = cbm_error_ranges_str(a, ®s); + /* One range covering nearly the whole file is not advice, it is + * noise: "look at lines 1 to 13047" of a 13046-line file tells a + * reader nothing they did not already know. Mark those separately + * so the report can say "read the source" instead. See + * parse_unusable in cbm.h for which files land here and why. */ + if (regs.count == 1 && regs.dropped == 0) { + uint32_t total = cbm_count_lines(source, source_len); + uint32_t span = regs.ends[0] - regs.starts[0] + 1; + if (total > 0 && span * 100 >= total * CBM_UNUSABLE_PCT) { + result->parse_unusable = true; + } + } } } diff --git a/internal/cbm/cbm.h b/internal/cbm/cbm.h index 4f06bebb7..921b8af70 100644 --- a/internal/cbm/cbm.h +++ b/internal/cbm/cbm.h @@ -511,6 +511,21 @@ typedef struct CBMFileResult { * completeness guarantee. Callers should treat a flagged file as "prefer * grep here", never treat an unflagged file as provably complete. */ bool parse_incomplete; + /* True when the ranges cover so much of the file that they are no longer + * useful advice — one range over 80% of the line count. The file WAS + * indexed, but pointing a reader at almost every line tells them nothing, + * so the report says "read the source" instead of listing the range. + * + * Its main customers are non-C languages. The refinement that narrows a + * whole-file range using the preprocessed parse only runs for C, C++ and + * CUDA, so a Python, Java or Ruby file whose root node is ERROR still + * reports 1-N. + * + * Note the naming: this field and the phase string it produces are both + * `parse_unusable`. The older `parse_incomplete` field emits the phase + * `parse_partial` instead. That mismatch is historical, not deliberate — + * do not copy it. */ + bool parse_unusable; const char *error_ranges; int error_region_count; bool is_test_file; diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 73534404a..38e518214 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -387,9 +387,11 @@ static const tool_def_t TOOLS[] = { "across projects to create CROSS_HTTP_CALLS/CROSS_ASYNC_CALLS/CROSS_CHANNEL edges. " "Requires target_projects param. Ensure target projects have fresh indexes first. " "COVERAGE: the response reports files that were NOT fully indexed — 'skipped' (not " - "indexed at all: oversized/read/parse failures) and 'parse_partial' (indexed, but " + "indexed at all: oversized/read/parse failures), 'parse_partial' (indexed, but " "constructs inside the listed line ranges could not be parsed and MAY be missing from " - "the graph). The embedded lists carry counts plus a FEW EXAMPLES only; the complete " + "the graph) and 'parse_unusable' (indexed, but the parse failed across nearly the whole " + "file, so read the source rather than any range). The embedded lists carry counts plus a " + "FEW EXAMPLES only; the complete " "lists are in the per-run 'logfile' (path in the response) and queryable any time via " "index_status or structurally via query_graph(graph=\"missed\"). Both signals are " "best-effort: absence of a flag is NOT a completeness guarantee; prefer grep inside " @@ -490,7 +492,8 @@ static const tool_def_t TOOLS[] = { "file structure of ONLY the files the indexer could NOT fully index (Project → Folder → " "File nodes with CONTAINS_FOLDER/CONTAINS_FILE edges; each File carries kind " "(\"parse_partial\" = indexed but constructs in the flagged line ranges MAY be missing; " - "or a skip phase) and detail (the line ranges / reason)). Example: MATCH (f:File) WHERE " + "\"parse_unusable\" = indexed but the ranges cover nearly the whole file, so read the " + "source; or a skip phase) and detail (the line ranges / reason)). Example: MATCH (f:File) WHERE " "f.kind = \\\"parse_partial\\\" RETURN f.file_path, f.detail. Absence from this graph is " "NOT a completeness guarantee.", "{\"type\":\"object\",\"properties\":{\"query\":{\"type\":\"string\",\"description\":\"Cypher " @@ -658,6 +661,8 @@ static const tool_def_t TOOLS[] = { "indexing-COVERAGE report — which files the indexer could NOT fully cover (best-effort " "signal): 'parse_partial' files WERE indexed but contain line ranges tree-sitter could not " "parse — constructs there MAY be missing from the graph (some are still recovered); " + "'parse_unusable' files WERE indexed too, but one range covers 80 percent or more of the file, so " + "the ranges are useless advice — read the source; " "'skipped' files were not indexed at all (oversized/read/parse failure). Use this before " "trusting graph completeness on a file: if a file is listed, ALSO grep it (especially the " "flagged ranges). IMPORTANT: absence from these lists is NOT a completeness guarantee — the " @@ -4564,10 +4569,12 @@ static void add_coverage_report(yyjson_mut_doc *doc, yyjson_mut_val *root, cbm_s (void)cbm_store_coverage_get(store, project, &rows, &count); yyjson_mut_val *pp_files = yyjson_mut_arr(doc); + yyjson_mut_val *pu_files = yyjson_mut_arr(doc); yyjson_mut_val *sk_files = yyjson_mut_arr(doc); yyjson_mut_val *ni_dirs = yyjson_mut_arr(doc); yyjson_mut_val *ni_files = yyjson_mut_arr(doc); int pp_n = 0; + int pu_n = 0; int sk_n = 0; int ni_dir_n = 0; int ni_file_n = 0; @@ -4582,6 +4589,18 @@ static void add_coverage_report(yyjson_mut_doc *doc, yyjson_mut_val *root, cbm_s yyjson_mut_arr_add_val(pp_files, fe); } pp_n++; + } else if (strcmp(kind, "parse_unusable") == 0) { + /* Needs its own branch. The catch-all below builds skipped[], and + * a reader who finds a file there believes it was never indexed. */ + if (pu_n < COVERAGE_FILE_CAP) { + yyjson_mut_val *fe = yyjson_mut_obj(doc); + yyjson_mut_obj_add_strcpy(doc, fe, "path", rows[i].rel_path); + yyjson_mut_obj_add_bool(doc, fe, "whole_file", true); + const char *dash = rows[i].detail ? strchr(rows[i].detail, '-') : NULL; + yyjson_mut_obj_add_int(doc, fe, "lines", dash ? atoi(dash + 1) : 0); + yyjson_mut_arr_add_val(pu_files, fe); + } + pu_n++; } else if (strcmp(kind, "not_indexed_dir") == 0) { if (ni_dir_n < COVERAGE_FILE_CAP) { yyjson_mut_arr_add_strcpy(doc, ni_dirs, rows[i].rel_path); @@ -4614,6 +4633,14 @@ static void add_coverage_report(yyjson_mut_doc *doc, yyjson_mut_val *root, cbm_s yyjson_mut_obj_add_bool(doc, pp, "truncated", pp_n > COVERAGE_FILE_CAP); yyjson_mut_obj_add_val(doc, root, "parse_partial", pp); + /* Indexed, but the parse failed across nearly the whole file, so naming + * line ranges helps nobody — read the source instead. */ + yyjson_mut_val *pu = yyjson_mut_obj(doc); + yyjson_mut_obj_add_val(doc, pu, "files", pu_files); + yyjson_mut_obj_add_int(doc, pu, "count", pu_n); + yyjson_mut_obj_add_bool(doc, pu, "truncated", pu_n > COVERAGE_FILE_CAP); + yyjson_mut_obj_add_val(doc, root, "parse_unusable", pu); + yyjson_mut_val *sk = yyjson_mut_obj(doc); yyjson_mut_obj_add_val(doc, sk, "files", sk_files); yyjson_mut_obj_add_int(doc, sk, "count", sk_n); @@ -4656,7 +4683,8 @@ enum { COVERAGE_SCOPE_MAX = 32, COVERAGE_SCOPE_DEFAULT_LIMIT = 200, COVERAGE_SCOPE_MAX_LIMIT = 1000, - COVERAGE_RANGE_MAX = 128, + COVERAGE_RANGE_MAX = 256, /* matches CBM_MAX_ERROR_REGIONS — a lower value here + would just move the silent clip downstream */ }; bool cbm_path_within_root(const char *root_path, const char *abs_path); /* defined below */ @@ -4768,6 +4796,14 @@ static const char *coverage_path_freshness(cbm_store_t *store, const char *proje return matches ? "metadata_match" : "metadata_changed"; } +/* Read an "start-end,start-end,...[,+]" string into a JSON ranges array. + * + * The optional trailing "+" says the producer's own cap threw N ranges away. + * Without reading it, a clipped list arrives here looking complete: the loop + * below stops at the '+' with no error and no leftover, so the row would claim + * a short, tidy set of ranges that is in fact missing entries. Set + * "truncated": true whenever ranges were lost — either by that marker, or by + * COVERAGE_RANGE_MAX stopping this loop. */ static void coverage_add_ranges(yyjson_mut_doc *doc, yyjson_mut_val *row, const char *detail) { if (!detail || !detail[0]) { return; @@ -4775,10 +4811,15 @@ static void coverage_add_ranges(yyjson_mut_doc *doc, yyjson_mut_val *row, const yyjson_mut_val *ranges = yyjson_mut_arr(doc); const char *p = detail; int emitted = 0; + bool truncated = false; while (*p && emitted < COVERAGE_RANGE_MAX) { while (*p == ' ' || *p == ',') { p++; } + if (*p == '+') { + truncated = true; /* the producer's cap dropped ranges we never saw */ + break; + } if (!isdigit((unsigned char)*p)) { break; } @@ -4810,9 +4851,15 @@ static void coverage_add_ranges(yyjson_mut_doc *doc, yyjson_mut_val *row, const break; } } + if (emitted >= COVERAGE_RANGE_MAX && *p) { + truncated = true; /* our own limit stopped the loop with input left over */ + } if (emitted > 0) { yyjson_mut_obj_add_val(doc, row, "ranges", ranges); } + if (truncated) { + yyjson_mut_obj_add_bool(doc, row, "truncated", true); + } } static void coverage_add_row_json(yyjson_mut_doc *doc, yyjson_mut_val *array, @@ -4826,7 +4873,8 @@ static void coverage_add_row_json(yyjson_mut_doc *doc, yyjson_mut_val *array, doc, item, "match", row->rel_path && strcmp(row->rel_path, requested_path) == 0 ? "exact" : "ancestor"); } - if (row->kind && strcmp(row->kind, "parse_partial") == 0) { + if (row->kind && (strcmp(row->kind, "parse_partial") == 0 || + strcmp(row->kind, "parse_unusable") == 0)) { coverage_add_ranges(doc, item, row->detail); } yyjson_mut_arr_add_val(array, item); @@ -4852,6 +4900,12 @@ static const char *coverage_status(const cbm_coverage_row_t *rows, int count, continue; } const char *kind = rows[i].kind ? rows[i].kind : ""; + /* "parse_unusable" must be named here. Without its own case it + * falls through to the catch-all below and reports "skipped", + * which is wrong in the way that matters: the file WAS indexed. */ + if (pass == 0 && strcmp(kind, "parse_unusable") == 0) { + return "unusable"; + } if (pass == 0 && strcmp(kind, "parse_partial") == 0) { return "partial"; } @@ -4879,6 +4933,11 @@ static const char *coverage_recommended_action(const char *status, const char *f if (strcmp(status, "partial") == 0) { return "read_ranges_and_verify_scope"; } + if (strcmp(status, "unusable") == 0) { + /* The ranges cover nearly the whole file, so sending a reader to them + * is the same as sending them to the file. Say the useful thing. */ + return "read_source_directly"; + } if (strcmp(status, "skipped") == 0) { return "read_source_directly"; } @@ -7856,6 +7915,19 @@ static bool is_parse_partial(const cbm_file_error_t *e) { return e->phase && strcmp(e->phase, "parse_partial") == 0; } +/* The same, for the whole-file variant: one range covers 80% or more of the + * file, so listing the lines is useless advice. Also indexed, also not a skip. */ +static bool is_parse_unusable(const cbm_file_error_t *e) { + return e->phase && strcmp(e->phase, "parse_unusable") == 0; +} + +/* Either coverage phase. Both mean the file WAS indexed, so both must stay out + * of skipped[] — a reader who sees a file there believes it is absent from the + * graph entirely. */ +static bool is_parse_coverage(const cbm_file_error_t *e) { + return is_parse_partial(e) || is_parse_unusable(e); +} + /* Attach a summary of per-file skips (Stage 2 / Track B). Always emits a * top-level "skipped_count" (0 on clean runs) so consumers can rely on it. * When there are skips, also emits: @@ -7863,13 +7935,13 @@ static bool is_parse_partial(const cbm_file_error_t *e) { * and, if a per-run logfile was written, "logfile": "". * The run status stays "indexed" — a skipped file is the expected handled * outcome, not a failure. errs[] is borrowed (copied into doc) and may contain - * parse_partial entries, which are filtered out here (reported separately by - * add_parse_partial_summary). */ + * parse_partial and parse_unusable entries, which are filtered out here (both + * reported separately by add_parse_partial_summary). */ static void add_skipped_summary(yyjson_mut_doc *doc, yyjson_mut_val *root, const cbm_file_error_t *errs, int count, const char *logfile) { int skips = 0; for (int i = 0; i < count; i++) { - if (!is_parse_partial(&errs[i])) { + if (!is_parse_coverage(&errs[i])) { skips++; } } @@ -7884,7 +7956,7 @@ static void add_skipped_summary(yyjson_mut_doc *doc, yyjson_mut_val *root, yyjson_mut_val *files = yyjson_mut_arr(doc); int shown = 0; for (int i = 0; i < count && shown < INDEX_SKIPPED_FILE_CAP; i++) { - if (is_parse_partial(&errs[i])) { + if (is_parse_coverage(&errs[i])) { continue; } yyjson_mut_val *fe = yyjson_mut_obj(doc); @@ -7943,6 +8015,53 @@ static void add_parse_partial_summary(yyjson_mut_doc *doc, yyjson_mut_val *root, yyjson_mut_obj_add_val(doc, root, "parse_partial", pp); } +/* Attach the whole-file half of the coverage summary. Always emits a top-level + * "parse_unusable_count" (0 on clean runs) so the CI coverage gate can read it + * without parsing anything else. When files were flagged: + * "parse_unusable": {"files":[{path,lines,whole_file}..(<=50)], "count":N, + * "truncated":bool, "note":"..."} + * + * These files WERE indexed, exactly like parse_partial ones. The difference is + * that their range covers 80% or more of the file, so the range is not worth + * printing — "lines" gives the size and "whole_file" says plainly that reading + * the ranges is the same as reading the file. */ +static void add_parse_unusable_summary(yyjson_mut_doc *doc, yyjson_mut_val *root, + const cbm_file_error_t *errs, int count) { + int unusable = 0; + for (int i = 0; i < count; i++) { + if (is_parse_unusable(&errs[i])) { + unusable++; + } + } + yyjson_mut_obj_add_int(doc, root, "parse_unusable_count", unusable); + if (!errs || unusable <= 0) { + return; + } + yyjson_mut_val *pu = yyjson_mut_obj(doc); + yyjson_mut_val *files = yyjson_mut_arr(doc); + int shown = 0; + for (int i = 0; i < count && shown < INDEX_SKIPPED_FILE_CAP; i++) { + if (!is_parse_unusable(&errs[i])) { + continue; + } + yyjson_mut_val *fe = yyjson_mut_obj(doc); + yyjson_mut_obj_add_strcpy(doc, fe, "path", errs[i].path ? errs[i].path : ""); + yyjson_mut_obj_add_bool(doc, fe, "whole_file", true); + /* The range string is "start-end"; its end line is the file length. */ + const char *dash = errs[i].reason ? strchr(errs[i].reason, '-') : NULL; + yyjson_mut_obj_add_int(doc, fe, "lines", dash ? atoi(dash + 1) : 0); + yyjson_mut_arr_add_val(files, fe); + shown++; + } + yyjson_mut_obj_add_val(doc, pu, "files", files); + yyjson_mut_obj_add_int(doc, pu, "count", unusable); + yyjson_mut_obj_add_bool(doc, pu, "truncated", unusable > INDEX_SKIPPED_FILE_CAP); + yyjson_mut_obj_add_str(doc, pu, "note", + "Indexed, but the parse failed across nearly the whole file, so line " + "ranges are not useful here — read the source directly."); + yyjson_mut_obj_add_val(doc, root, "parse_unusable", pu); +} + /* The pipeline persists the complete current coverage set before this * response is built. Prefer that set over the per-run errors so incremental * runs that do not revisit a flagged file, and artifact bootstraps, do not @@ -7986,6 +8105,7 @@ static bool add_persisted_failure_summaries(yyjson_mut_doc *doc, yyjson_mut_val add_skipped_summary(doc, root, failures, failure_count, logfile); add_parse_partial_summary(doc, root, failures, failure_count); + add_parse_unusable_summary(doc, root, failures, failure_count); free(failures); cbm_store_free_coverage(rows, row_count); return true; @@ -8063,6 +8183,7 @@ static bool build_index_success_response(cbm_mcp_server_t *srv, yyjson_mut_doc * if (!store || !add_persisted_failure_summaries(doc, root, store, project_name, logfile)) { add_skipped_summary(doc, root, file_errors, file_error_count, logfile); add_parse_partial_summary(doc, root, file_errors, file_error_count); + add_parse_unusable_summary(doc, root, file_errors, file_error_count); } int nodes = 0; int edges = 0; @@ -9197,10 +9318,10 @@ static void add_string_array(yyjson_mut_doc *doc, yyjson_mut_val *obj, const cha } /* get_code_snippet coverage note (#963): if the resolved node's file is - * flagged parse_partial, warn that the graph may under-report this file. - * Correlated by construction — the result names its file. (An entirely- - * skipped file cannot appear here: it has no nodes to resolve a snippet - * from.) */ + * flagged parse_partial or parse_unusable, warn that the graph may + * under-report this file. Correlated by construction — the result names its + * file. (An entirely-skipped file cannot appear here: it has no nodes to + * resolve a snippet from.) */ static void add_snippet_coverage_note(yyjson_mut_doc *doc, yyjson_mut_val *root_obj, cbm_store_t *store, const cbm_node_t *node) { if (!node->file_path || !node->file_path[0] || !node->project) { @@ -9213,18 +9334,28 @@ static void add_snippet_coverage_note(yyjson_mut_doc *doc, yyjson_mut_val *root_ return; } for (int i = 0; i < count; i++) { - if (rows[i].rel_path && strcmp(rows[i].rel_path, node->file_path) == 0 && rows[i].kind && - strcmp(rows[i].kind, "parse_partial") == 0) { - char note[CBM_SZ_1K]; + if (!rows[i].rel_path || strcmp(rows[i].rel_path, node->file_path) != 0 || !rows[i].kind) { + continue; + } + char note[CBM_SZ_1K]; + if (strcmp(rows[i].kind, "parse_unusable") == 0) { + snprintf(note, sizeof(note), + "The parse of this file failed across nearly the whole of it, so most " + "constructs are missing from the graph and naming line ranges would not " + "help. Read the source directly — the source above is ground truth. " + "(best-effort signal)"); + } else if (strcmp(rows[i].kind, "parse_partial") == 0) { snprintf(note, sizeof(note), "This file was only PARTIALLY indexed — line range(s) %s could not be " "parsed, so constructs there may be missing from the graph (callers/callees " "and search results can under-report this file). The source above is ground " "truth. (best-effort signal)", rows[i].detail && rows[i].detail[0] ? rows[i].detail : "?"); - yyjson_mut_obj_add_strcpy(doc, root_obj, "coverage_note", note); - break; + } else { + continue; } + yyjson_mut_obj_add_strcpy(doc, root_obj, "coverage_note", note); + break; } cbm_store_free_coverage(rows, count); } diff --git a/src/pipeline/pass_definitions.c b/src/pipeline/pass_definitions.c index 7ac98e9cd..b798cda5e 100644 --- a/src/pipeline/pass_definitions.c +++ b/src/pipeline/pass_definitions.c @@ -18,6 +18,7 @@ enum { PD_RING = 4, PD_RING_MASK = 3, PD_JSON_MARGIN = 10, PD_ESC_MARGIN = 3, PD enum { PD_JSON_FIELD_OVERHEAD = 6 }; #include "pipeline/pipeline.h" #include +#include #include "pipeline/pipeline_internal.h" #include "graph_buffer/graph_buffer.h" #include "foundation/log.h" @@ -544,23 +545,79 @@ static bool objectscript_export_append_secondary_arrays(CBMFileResult *aggregate /* Preserve every generated class's parse diagnostics. The generated UDL * snippets all map back to one physical Studio Export file, so their compact * range lists can be concatenated using the ordinary comma separator. */ +/* Read the trailing ",+" truncation marker off a range string. Returns the + * number of dropped ranges the marker reports, or 0 when there is no marker, + * and writes the length of the part before the marker to `body_len`. */ +static int objectscript_export_split_range_marker(const char *ranges, size_t *body_len) { + size_t len = ranges ? strlen(ranges) : 0; + *body_len = len; + if (len == 0) { + return 0; + } + size_t i = len; + while (i > 0 && isdigit((unsigned char)ranges[i - 1])) { + i--; + } + if (i == len || i == 0 || ranges[i - 1] != '+') { + return 0; + } + size_t marker = i - 1; /* index of '+' */ + if (marker > 0 && ranges[marker - 1] == ',') { + marker--; /* drop the separator too */ + } + *body_len = marker; + return atoi(ranges + i); +} + +/* Join one Studio Export part's ranges onto the aggregate. + * + * One export file can hold several elements, each parsed separately, + * so their range strings get concatenated. A ",+" truncation marker must + * end up ONCE, at the very end: every reader stops at the first token that is + * not a range, so a marker left in the middle would silently hide every range + * after it. Strip the marker off both sides, join the plain ranges, then add + * one marker back carrying the summed count. */ static bool objectscript_export_append_error_ranges(CBMFileResult *aggregate, const CBMFileResult *part) { aggregate->parse_incomplete = aggregate->parse_incomplete || part->parse_incomplete; + aggregate->parse_unusable = aggregate->parse_unusable || part->parse_unusable; aggregate->error_region_count += part->error_region_count; if (!part->error_ranges || !part->error_ranges[0]) { return true; } + + size_t agg_len = 0; + size_t part_len = 0; + int dropped = 0; + const char *agg_body = aggregate->error_ranges; + if (agg_body && agg_body[0]) { + dropped += objectscript_export_split_range_marker(agg_body, &agg_len); + } else { + agg_body = NULL; + } + dropped += objectscript_export_split_range_marker(part->error_ranges, &part_len); + const char *combined = NULL; - if (aggregate->error_ranges && aggregate->error_ranges[0]) { - combined = cbm_arena_sprintf(&aggregate->arena, "%s,%s", aggregate->error_ranges, - part->error_ranges); + if (agg_body && agg_len > 0 && part_len > 0) { + combined = cbm_arena_sprintf(&aggregate->arena, "%.*s,%.*s", (int)agg_len, agg_body, + (int)part_len, part->error_ranges); + } else if (agg_body && agg_len > 0) { + combined = cbm_arena_sprintf(&aggregate->arena, "%.*s", (int)agg_len, agg_body); + } else if (part_len > 0) { + combined = cbm_arena_sprintf(&aggregate->arena, "%.*s", (int)part_len, part->error_ranges); } else { - combined = cbm_arena_strdup(&aggregate->arena, part->error_ranges); + combined = cbm_arena_strdup(&aggregate->arena, ""); } if (!combined) { return false; } + if (dropped > 0) { + combined = cbm_arena_sprintf(&aggregate->arena, "%s%s+%d", combined, + combined[0] ? "," : "", dropped); + if (!combined) { + return false; + } + } aggregate->error_ranges = combined; return true; } @@ -787,7 +844,8 @@ int cbm_pipeline_pass_definitions(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t * ERROR/MISSING regions — see pass_parallel.c (keep in sync). */ cbm_pipeline_add_file_error(ctx->pipeline, rel, result->error_ranges ? result->error_ranges : "unknown", - "parse_partial"); + result->parse_unusable ? "parse_unusable" + : "parse_partial"); } /* Create nodes for each definition */ diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index 1ce766927..a75985fc3 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -889,11 +889,12 @@ static void extract_worker(int worker_id, void *ctx_ptr) { } else if (result->parse_incomplete) { /* Best-effort parse-coverage signal (#963): the file WAS indexed, * but its tree contains ERROR/MISSING regions whose constructs are - * silently absent from the graph. Not a skip — recorded under the - * distinct "parse_partial" phase (reason = the line-range list) so - * the MCP layer reports it separately from skipped[]. */ + * silently absent from the graph. Neither phase is a skip — both + * are recorded separately from skipped[] by the MCP layer. + * "parse_unusable" means one range covers so much of the file that + * naming the lines helps nobody; see parse_unusable in cbm.h. */ pp_err_add(errs, fi->rel_path, result->error_ranges ? result->error_ranges : "unknown", - "parse_partial"); + result->parse_unusable ? "parse_unusable" : "parse_partial"); } /* Create definition nodes in local gbuf */ diff --git a/src/store/store.c b/src/store/store.c index 03e42b0ed..0f0c58eeb 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -300,9 +300,25 @@ static int init_schema(cbm_store_t *s) { " PRIMARY KEY (project, rel_path)" ");" /* Best-effort indexing-coverage signal (#963). One row per file the - * indexer could not fully cover: kind "parse_partial" (indexed, but the - * parse tree had ERROR/MISSING regions — detail = 1-based line ranges) - * or a skip phase ("read"/"extract"/"oversized" — detail = reason). + * indexer could not fully cover. `kind` says which of three things + * happened, and `detail` means something different in each: + * + * "parse_partial" the file WAS indexed, but the parse tree had + * ERROR/MISSING regions. detail = 1-based line + * ranges, "start-end,start-end", with an optional + * trailing "+" saying N more ranges were dropped + * by the producer's cap. Read those lines. + * "parse_unusable" the file WAS indexed, but one range covers 80% or + * more of it, so naming the lines is useless advice. + * detail = the same range string. Read the source. + * a skip phase the file was NOT indexed at all: "read", + * "extract" or "oversized". detail = the reason. + * + * The first two are easy to confuse with the third, and the difference + * matters to a reader: a skipped file is absent from the graph, while + * the other two are present but incomplete. Name a new kind so that + * distinction stays obvious — "parse_failed" would read as a skip. + * * Deliberately SEPARATE from the graph tables: coverage is metadata * about the graph, not part of it. */ "CREATE TABLE IF NOT EXISTS index_coverage (" diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index 0c913837b..3d2cb2998 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -33,6 +33,7 @@ #include #include #include +#include /* Convenience extract wrapper (same shape as test_extraction_imports.c). */ static CBMFileResult *do_extract(const char *src, CBMLanguage lang, const char *path) { @@ -237,12 +238,22 @@ TEST(py_clean_file_not_flagged) { PASS(); } +/* Read the trailing "+" truncation marker off a range string. Returns N, or + * 0 when the string carries no marker. */ +static int ranges_dropped_marker(const char *ranges) { + const char *plus = ranges ? strrchr(ranges, '+') : NULL; + if (!plus || !isdigit((unsigned char)plus[1])) { + return 0; + } + return atoi(plus + 1); +} + TEST(error_region_cap_is_honored) { /* Pathological input: many separate unrecoverable garbage blocks * interleaved with valid defs. The collector must stay bounded by its - * 64-region cap (matches CBM_MAX_ERROR_REGIONS in cbm.c) — pathological + * 256-region cap (matches CBM_MAX_ERROR_REGIONS in cbm.c) — pathological * input can't blow up the report, and the flag itself still fires. */ - enum { GARBAGE_BLOCKS = 200, LINE_CAP = 64 }; + enum { GARBAGE_BLOCKS = 400, LINE_CAP = 256 }; char *src = (char *)malloc(GARBAGE_BLOCKS * 96 + 1); ASSERT_NOT_NULL(src); size_t off = 0; @@ -261,6 +272,55 @@ TEST(error_region_cap_is_honored) { PASS(); } +/* A clipped range list must say so. 400 garbage blocks overrun the 256-region + * cap, so the report keeps 256 ranges and ends with a "+" marker naming the + * number thrown away. Without the marker the short list reads as a complete + * one, which is the whole defect this guards. */ +TEST(error_region_cap_reports_what_it_dropped) { + enum { GARBAGE_BLOCKS = 400, LINE_CAP = 256 }; + char *src = (char *)malloc(GARBAGE_BLOCKS * 96 + 1); + ASSERT_NOT_NULL(src); + size_t off = 0; + for (int i = 0; i < GARBAGE_BLOCKS; i++) { + off += (size_t)snprintf( + src + off, 96, "def ok%d():\n return %d\n%%%%%% garbage%d ((( %%%%%%\n", i, i, i); + } + CBMFileResult *r = do_extract(src, CBM_LANG_PYTHON, "cap_marker.py"); + free(src); + ASSERT_NOT_NULL(r); + ASSERT_NOT_NULL(r->error_ranges); + /* The cap bound, so the kept list is full and the marker is present. */ + ASSERT_EQ(r->error_region_count, LINE_CAP); + int dropped = ranges_dropped_marker(r->error_ranges); + ASSERT_GTE(dropped, 1); + /* Every block produces at most one region, so the total cannot exceed the + * number of blocks — a marker that overcounts would fail here. */ + ASSERT_LTE(r->error_region_count + dropped, GARBAGE_BLOCKS); + /* The marker is a SUFFIX: nothing follows it, or a reader stops early and + * silently loses every range after it. */ + const char *plus = strrchr(r->error_ranges, '+'); + ASSERT_NOT_NULL(plus); + for (const char *c = plus + 1; *c; c++) { + ASSERT_TRUE(isdigit((unsigned char)*c)); + } + cbm_free_result(r); + PASS(); +} + +/* Inverse guard: a file that stays under the cap must carry NO marker, or + * every ordinary report would look clipped. */ +TEST(uncapped_ranges_carry_no_marker) { + const char *src = "def ok():\n return 1\n%%% garbage (((\ndef ok2():\n return 2\n"; + CBMFileResult *r = do_extract(src, CBM_LANG_PYTHON, "small.py"); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_NOT_NULL(r->error_ranges); + ASSERT_EQ(ranges_dropped_marker(r->error_ranges), 0); + ASSERT_NULL(strchr(r->error_ranges, '+')); + cbm_free_result(r); + PASS(); +} + /* Trailing recovered functions AFTER the failed #ifdef region must not * unflag it: recovery evidence must originate INSIDE the region, and the * unrecovered lines (the first branch's `guarded`) keep it flagged. */ @@ -604,6 +664,62 @@ TEST(c_clean_file_stays_unflagged_after_refinement) { } +/* The whole-file class, and the reason the parse_unusable kind exists. + * + * The Phase 2 refinement that narrows a whole-file range using the + * preprocessed parse only runs for C, C++ and CUDA. A Python file whose root + * node is ERROR gets no such help, so it still reports one range covering + * every line — and one range over 80% of a file is not advice worth printing. */ +TEST(python_whole_file_error_is_unusable) { + const char *src = ")))\n((( \n]]] [[[\ndef x(:\n"; + CBMFileResult *r = do_extract(src, CBM_LANG_PYTHON, "unparseable.py"); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_TRUE(r->parse_unusable); + ASSERT_EQ(r->error_region_count, 1); + ASSERT_NOT_NULL(r->error_ranges); + cbm_free_result(r); + PASS(); +} + +/* Inverse guard, and the one that keeps the kind meaningful: a file with a + * real but LOCAL parse failure must stay parse_partial. If this flipped, every + * flagged file would say "read the source" and the ranges would stop earning + * their keep. */ +TEST(local_error_stays_partial_not_unusable) { + const char *src = "def ok():\n return 1\n%%% garbage (((\ndef ok2():\n return 2\n" + "def ok3():\n return 3\ndef ok4():\n return 4\n" + "def ok5():\n return 5\ndef ok6():\n return 6\n"; + CBMFileResult *r = do_extract(src, CBM_LANG_PYTHON, "local_error.py"); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_FALSE(r->parse_unusable); + cbm_free_result(r); + PASS(); +} + +/* A clean file is neither. */ +TEST(clean_file_is_neither_partial_nor_unusable) { + CBMFileResult *r = do_extract(C_CLEAN, CBM_LANG_C, "clean_kinds.c"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->parse_incomplete); + ASSERT_FALSE(r->parse_unusable); + cbm_free_result(r); + PASS(); +} + +/* The C file that started this work must NOT land in the unusable class. Its + * whole-file range is exactly what Phase 2 broke up, so if this ever flips + * back to true the refinement has stopped working. */ +TEST(c_ifdef_split_is_partial_never_unusable) { + CBMFileResult *r = do_extract(C_IFDEF_SPLIT, CBM_LANG_C, "split_kind.c"); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_FALSE(r->parse_unusable); + cbm_free_result(r); + PASS(); +} + SUITE(parse_coverage) { RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete); RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted); @@ -613,6 +729,12 @@ SUITE(parse_coverage) { RUN_TEST(py_recovered_def_not_flagged); RUN_TEST(py_clean_file_not_flagged); RUN_TEST(error_region_cap_is_honored); + RUN_TEST(error_region_cap_reports_what_it_dropped); + RUN_TEST(uncapped_ranges_carry_no_marker); + RUN_TEST(python_whole_file_error_is_unusable); + RUN_TEST(local_error_stays_partial_not_unusable); + RUN_TEST(clean_file_is_neither_partial_nor_unusable); + RUN_TEST(c_ifdef_split_is_partial_never_unusable); RUN_TEST(c_trailing_recovered_defs_keep_flag); RUN_TEST(dockerfile_missing_final_newline_not_flagged_issue1610); RUN_TEST(dockerfile_with_final_newline_still_clean_issue1610); From 8f5b01014c4eb4afca3c0aaa439f8d4e931eefb7 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 01:35:52 -0400 Subject: [PATCH 3/9] test(coverage): pin the truncation marker, the partial ceiling and the grammar limit (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5. Four test groups, each checked RED before it was kept. - The Studio Export range join puts ONE ",+" marker at the end with the summed drop count. A marker left mid-string makes every reader stop there and silently lose the ranges after it. Reaching the join through the pipeline needs an export file with 256+ error regions across two elements, so it goes through a test seam, following the pattern already in this repo (CBM_COVERAGE_MARKER_TEST_API). - check_index_coverage emits every range in front of a marker, never turns the marker's digits into a range, and reports "truncated" from BOTH caps — the producer's and its own 256 limit. - test_index_resilience now has a ceiling beside its floor: exactly one of the two fixture files is flagged, the clean neighbour is absent, and the range does not cover the whole file. - The three _Thread_local forms are pinned as measured. Only the array form fails today; the plan's Phase 0 also listed the pointer form, and that is wrong on the grammar shipped now. Also fixes 13 clang-format violations the earlier commits on this branch left in cbm.c, mcp.c and pass_definitions.c. `make -f Makefile.cbm lint-format` would have failed CI. The changes are whitespace only — the two reflowed tool descriptions concatenate byte-identically, so no output moved. Full suite: 7735 passed, 28 failed, 7 skipped. The 28 are the pre-existing cli install/uninstall failures, identical at clean HEAD. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- Makefile.cbm | 4 +- internal/cbm/cbm.c | 7 ++- src/mcp/mcp.c | 11 +++-- src/pipeline/pass_definitions.c | 25 +++++++--- src/pipeline/pipeline_internal.h | 6 +++ tests/test_index_resilience.c | 16 +++++- tests/test_mcp.c | 81 ++++++++++++++++++++++++++++++ tests/test_parse_coverage.c | 46 ++++++++++++++++- tests/test_pipeline.c | 84 ++++++++++++++++++++++++++++++++ 9 files changed, 260 insertions(+), 20 deletions(-) diff --git a/Makefile.cbm b/Makefile.cbm index dc92e1703..778b6b234 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -98,9 +98,10 @@ endif KOTLIN_DEDUP_TEST_DEFINE = -DCBM_KOTLIN_DEDUP_TEST_API=1 CALL_REFERENCE_LOOKUP_TEST_DEFINE = -DCBM_CALL_REFERENCE_LOOKUP_TEST_API=1 INCREMENTAL_TEST_DEFINE = -DCBM_INCREMENTAL_TEST_API=1 +COVERAGE_MARKER_TEST_DEFINE = -DCBM_COVERAGE_MARKER_TEST_API=1 CFLAGS_TEST = $(CFLAGS_COMMON) $(EDITOR_TEST_DEFINES) $(SANITIZED_DEFINE) \ $(KOTLIN_DEDUP_TEST_DEFINE) $(CALL_REFERENCE_LOOKUP_TEST_DEFINE) \ - $(INCREMENTAL_TEST_DEFINE) -g -O1 $(SANITIZE) + $(INCREMENTAL_TEST_DEFINE) $(COVERAGE_MARKER_TEST_DEFINE) -g -O1 $(SANITIZE) CXXFLAGS_TEST = $(CXXFLAGS_COMMON) $(SANITIZED_DEFINE) -g -O1 $(SANITIZE) $(CXX_STDLIB_FLAGS) # TSan (can't combine with ASan) @@ -118,6 +119,7 @@ TSAN_SANITIZE = -fsanitize=thread -fno-omit-frame-pointer # macro of ours. CFLAGS_TSAN = $(CFLAGS_COMMON) $(EDITOR_TEST_DEFINES) $(KOTLIN_DEDUP_TEST_DEFINE) \ $(CALL_REFERENCE_LOOKUP_TEST_DEFINE) $(INCREMENTAL_TEST_DEFINE) \ + $(COVERAGE_MARKER_TEST_DEFINE) \ -DCBM_SANITIZED_BUILD=1 -g -O1 $(TSAN_SANITIZE) CXXFLAGS_TSAN = $(CXXFLAGS_COMMON) -DCBM_SANITIZED_BUILD=1 -g -O1 \ $(TSAN_SANITIZE) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index 049d1cd1e..c091561eb 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -1775,16 +1775,15 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua orig_lines++; } } - uint8_t *map = - (uint8_t *)cbm_arena_alloc(a, (size_t)orig_lines + 2); + uint8_t *map = (uint8_t *)cbm_arena_alloc(a, (size_t)orig_lines + 2); int exp_lines = preprocessed->expanded_line_count; uint8_t *bad_rows = exp_lines > 0 ? (uint8_t *)calloc((size_t)exp_lines + 2, 1) : NULL; if (map && bad_rows) { memset(map, 0, (size_t)orig_lines + 2); cbm_mark_no_code_lines(source, source_len, map, orig_lines); - cbm_mark_pp_error_rows(pp_root, bad_rows, (uint32_t)exp_lines, - expanded, expanded_len); + cbm_mark_pp_error_rows(pp_root, bad_rows, (uint32_t)exp_lines, expanded, + expanded_len); /* Walk the expanded text once. An expanded line * only vouches for its original line when it * actually HAS content: the preprocessor emits a diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 38e518214..ca78d8fa5 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -493,7 +493,8 @@ static const tool_def_t TOOLS[] = { "File nodes with CONTAINS_FOLDER/CONTAINS_FILE edges; each File carries kind " "(\"parse_partial\" = indexed but constructs in the flagged line ranges MAY be missing; " "\"parse_unusable\" = indexed but the ranges cover nearly the whole file, so read the " - "source; or a skip phase) and detail (the line ranges / reason)). Example: MATCH (f:File) WHERE " + "source; or a skip phase) and detail (the line ranges / reason)). " + "Example: MATCH (f:File) WHERE " "f.kind = \\\"parse_partial\\\" RETURN f.file_path, f.detail. Absence from this graph is " "NOT a completeness guarantee.", "{\"type\":\"object\",\"properties\":{\"query\":{\"type\":\"string\",\"description\":\"Cypher " @@ -661,8 +662,8 @@ static const tool_def_t TOOLS[] = { "indexing-COVERAGE report — which files the indexer could NOT fully cover (best-effort " "signal): 'parse_partial' files WERE indexed but contain line ranges tree-sitter could not " "parse — constructs there MAY be missing from the graph (some are still recovered); " - "'parse_unusable' files WERE indexed too, but one range covers 80 percent or more of the file, so " - "the ranges are useless advice — read the source; " + "'parse_unusable' files WERE indexed too, but one range covers 80 percent or more of " + "the file, so the ranges are useless advice — read the source; " "'skipped' files were not indexed at all (oversized/read/parse failure). Use this before " "trusting graph completeness on a file: if a file is listed, ALSO grep it (especially the " "flagged ranges). IMPORTANT: absence from these lists is NOT a completeness guarantee — the " @@ -4873,8 +4874,8 @@ static void coverage_add_row_json(yyjson_mut_doc *doc, yyjson_mut_val *array, doc, item, "match", row->rel_path && strcmp(row->rel_path, requested_path) == 0 ? "exact" : "ancestor"); } - if (row->kind && (strcmp(row->kind, "parse_partial") == 0 || - strcmp(row->kind, "parse_unusable") == 0)) { + if (row->kind && + (strcmp(row->kind, "parse_partial") == 0 || strcmp(row->kind, "parse_unusable") == 0)) { coverage_add_ranges(doc, item, row->detail); } yyjson_mut_arr_add_val(array, item); diff --git a/src/pipeline/pass_definitions.c b/src/pipeline/pass_definitions.c index b798cda5e..7c7ed6884 100644 --- a/src/pipeline/pass_definitions.c +++ b/src/pipeline/pass_definitions.c @@ -561,9 +561,9 @@ static int objectscript_export_split_range_marker(const char *ranges, size_t *bo if (i == len || i == 0 || ranges[i - 1] != '+') { return 0; } - size_t marker = i - 1; /* index of '+' */ + size_t marker = i - 1; /* index of '+' */ if (marker > 0 && ranges[marker - 1] == ',') { - marker--; /* drop the separator too */ + marker--; /* drop the separator too */ } *body_len = marker; return atoi(ranges + i); @@ -612,8 +612,8 @@ static bool objectscript_export_append_error_ranges(CBMFileResult *aggregate, return false; } if (dropped > 0) { - combined = cbm_arena_sprintf(&aggregate->arena, "%s%s+%d", combined, - combined[0] ? "," : "", dropped); + combined = cbm_arena_sprintf(&aggregate->arena, "%s%s+%d", combined, combined[0] ? "," : "", + dropped); if (!combined) { return false; } @@ -622,6 +622,16 @@ static bool objectscript_export_append_error_ranges(CBMFileResult *aggregate, return true; } +#if defined(CBM_COVERAGE_MARKER_TEST_API) && CBM_COVERAGE_MARKER_TEST_API +/* Test seam. This join only fires for a Studio Export file holding several + * elements where a class overruns the 256-region cap — hard to reach + * through the pipeline, easy to get wrong, and a wrong result hides ranges + * without saying so. Expose the join so the marker rules can be pinned. */ +bool cbm_pipeline_coverage_marker_test_join(CBMFileResult *aggregate, const CBMFileResult *part) { + return objectscript_export_append_error_ranges(aggregate, part); +} +#endif + /* Studio Export files may contain multiple elements, while the * pipeline cache has one slot per physical file. Extract each generated UDL * class independently (preserving the upstream parser behavior), then compose @@ -842,10 +852,9 @@ int cbm_pipeline_pass_definitions(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t } else if (result->parse_incomplete) { /* Best-effort parse-coverage signal (#963): indexed, but with * ERROR/MISSING regions — see pass_parallel.c (keep in sync). */ - cbm_pipeline_add_file_error(ctx->pipeline, rel, - result->error_ranges ? result->error_ranges : "unknown", - result->parse_unusable ? "parse_unusable" - : "parse_partial"); + cbm_pipeline_add_file_error( + ctx->pipeline, rel, result->error_ranges ? result->error_ranges : "unknown", + result->parse_unusable ? "parse_unusable" : "parse_partial"); } /* Create nodes for each definition */ diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index 45b0baf6b..d749c695a 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -864,6 +864,12 @@ void cbm_pp_bp_nap_cycles_reset(void); uint64_t cbm_pp_lsp_linear_fallback_rows(void); void cbm_pp_lsp_linear_fallback_rows_reset(void); +#if defined(CBM_COVERAGE_MARKER_TEST_API) && CBM_COVERAGE_MARKER_TEST_API +/* Test-only view of the Studio Export range join, so the ",+" truncation + * marker rules can be checked without building a 256-region export file. */ +bool cbm_pipeline_coverage_marker_test_join(CBMFileResult *aggregate, const CBMFileResult *part); +#endif + #if defined(CBM_CALL_REFERENCE_LOOKUP_TEST_API) && CBM_CALL_REFERENCE_LOOKUP_TEST_API /* Deterministic test-only operation count for the shared semantic-reference * matcher used by both sequential and fused-parallel usage materialization. */ diff --git a/tests/test_index_resilience.c b/tests/test_index_resilience.c index 14909a4e4..41bf29f2f 100644 --- a/tests/test_index_resilience.c +++ b/tests/test_index_resilience.c @@ -337,8 +337,16 @@ TEST(index_parse_partial_reported) { ASSERT_STR_EQ("indexed", status); ASSERT_EQ(yyjson_get_int(yyjson_obj_get(sc, "skipped_count")), 0); - /* The coverage signal is surfaced with ranges + the best-effort note. */ + /* The coverage signal is surfaced with ranges + the best-effort note. + * Both bounds matter. The floor catches the signal going missing. The + * ceiling catches the opposite failure: exactly one of the two files has + * a gap, so a count above 1 means the clean Python neighbour got flagged + * as well, which is how over-flagging looks from the outside. */ ASSERT_GTE(yyjson_get_int(yyjson_obj_get(sc, "parse_partial_count")), 1); + ASSERT_EQ(yyjson_get_int(yyjson_obj_get(sc, "parse_partial_count")), 1); + /* A local gap is not a whole-file failure, so the other coverage kind + * must stay empty here. */ + ASSERT_EQ(yyjson_get_int(yyjson_obj_get(sc, "parse_unusable_count")), 0); yyjson_val *pp = yyjson_obj_get(sc, "parse_partial"); ASSERT_NOT_NULL(pp); yyjson_val *files = yyjson_obj_get(pp, "files"); @@ -354,7 +362,13 @@ TEST(index_parse_partial_reported) { found_split = 1; ASSERT_NOT_NULL(ranges); ASSERT_GT((int)strlen(ranges), 0); + /* The gap is the two-header block, not the whole file. An + * 8-line file reported as 1-8 would be the old whole-file + * blame coming back. */ + ASSERT_NULL(strstr(ranges, "1-8")); } + /* The clean file must not appear in the list at all. */ + ASSERT_NULL(fp ? strstr(fp, "good.py") : NULL); } ASSERT_TRUE(found_split); const char *note = yyjson_get_str(yyjson_obj_get(pp, "note")); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index e8722ffc1..ac3081f20 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -2916,6 +2916,86 @@ TEST(tool_check_index_coverage_finds_path_beyond_status_cap) { PASS(); } +/* The range string can carry a trailing ",+" marker saying the producer hit + * its own cap and threw ranges away. The reader must emit every range in front + * of the marker, must not turn the marker itself into a range, and must say + * "truncated" so nobody reads a short list as a complete one. The reader has a + * second cap of its own, and that one must report itself the same way. */ +TEST(tool_check_index_coverage_reports_truncation_marker_issue963) { + enum { WIDE_RANGE_COUNT = 300 }; + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(st); + + const char *project = "coverage-marker"; + ASSERT_EQ(cbm_store_upsert_project(st, project, "/tmp/coverage-marker"), CBM_STORE_OK); + cbm_mcp_server_set_project(srv, project); + + /* 300 one-line ranges — past the reader's own 256 limit. */ + char *wide = calloc(1, WIDE_RANGE_COUNT * 12 + 1); + ASSERT_NOT_NULL(wide); + size_t off = 0; + for (int i = 0; i < WIDE_RANGE_COUNT; i++) { + off += (size_t)snprintf(wide + off, WIDE_RANGE_COUNT * 12 + 1 - off, "%s%d-%d", + i ? "," : "", i * 3 + 1, i * 3 + 1); + } + + cbm_coverage_row_t rows[3] = { + {.rel_path = "src/marked.c", .kind = "parse_partial", .detail = "3-4,9-9,+12"}, + {.rel_path = "src/plain.c", .kind = "parse_partial", .detail = "3-4,9-9"}, + {.rel_path = "src/wide.c", .kind = "parse_partial", .detail = wide}, + }; + for (int i = 0; i < 3; i++) { + ASSERT_EQ(cbm_store_upsert_file_hash(st, project, rows[i].rel_path, "fixture", i + 1, 10), + CBM_STORE_OK); + } + ASSERT_EQ(cbm_store_coverage_replace(st, project, rows, 3), CBM_STORE_OK); + + /* The marked file: both real ranges survive, the marker is flagged, and the + * "12" from the marker never becomes a range of its own. */ + char *marked = + cbm_mcp_handle_tool(srv, "check_index_coverage", + "{\"project\":\"coverage-marker\",\"paths\":[\"src/marked.c\"]}"); + ASSERT_NOT_NULL(marked); + char *marked_inner = extract_text_content(marked); + ASSERT_NOT_NULL(marked_inner); + ASSERT_NOT_NULL(strstr(marked_inner, "\"start\":3")); + ASSERT_NOT_NULL(strstr(marked_inner, "\"start\":9")); + ASSERT_NULL(strstr(marked_inner, "\"start\":12")); + ASSERT_NOT_NULL(strstr(marked_inner, "\"truncated\":true")); + free(marked_inner); + free(marked); + + /* The same ranges without a marker must NOT be reported as truncated. */ + char *plain = + cbm_mcp_handle_tool(srv, "check_index_coverage", + "{\"project\":\"coverage-marker\",\"paths\":[\"src/plain.c\"]}"); + ASSERT_NOT_NULL(plain); + char *plain_inner = extract_text_content(plain); + ASSERT_NOT_NULL(plain_inner); + ASSERT_NOT_NULL(strstr(plain_inner, "\"start\":3")); + ASSERT_NULL(strstr(plain_inner, "\"truncated\":true")); + free(plain_inner); + free(plain); + + /* The reader's own limit stops the list early, so it must say so even + * though the producer sent no marker. */ + char *widest = + cbm_mcp_handle_tool(srv, "check_index_coverage", + "{\"project\":\"coverage-marker\",\"paths\":[\"src/wide.c\"]}"); + ASSERT_NOT_NULL(widest); + char *wide_inner = extract_text_content(widest); + ASSERT_NOT_NULL(wide_inner); + ASSERT_NOT_NULL(strstr(wide_inner, "\"truncated\":true")); + free(wide_inner); + free(widest); + + free(wide); + cbm_mcp_server_free(srv); + PASS(); +} + TEST(tool_check_index_coverage_reports_paths_scopes_and_ranges) { char tmp[256]; cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp)); @@ -13851,6 +13931,7 @@ SUITE(mcp) { RUN_TEST(tool_query_graph_basic); RUN_TEST(tool_index_status_no_project); RUN_TEST(tool_check_index_coverage_finds_path_beyond_status_cap); + RUN_TEST(tool_check_index_coverage_reports_truncation_marker_issue963); RUN_TEST(tool_check_index_coverage_reports_paths_scopes_and_ranges); RUN_TEST(tool_check_index_coverage_preserves_multiple_scope_labels); RUN_TEST(tool_check_index_coverage_accepts_truncated_ignored_catalog_for_fresh_path_issue1613); diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index 3d2cb2998..695d41992 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -20,7 +20,7 @@ * GREEN (fixed): cbm_extract_file sets parse_incomplete=true iff the tree * contains ERROR/MISSING nodes, records the 1-based line * ranges of the TOP-MOST error regions ("start-end,..."), - * bounded by the 64-region cap, and clean files stay + * bounded by the 256-region cap, and clean files stay * completely unflagged (no false positives). * * BEST-EFFORT framing (must never be weakened the other way): a flag means @@ -720,6 +720,49 @@ TEST(c_ifdef_split_is_partial_never_unusable) { PASS(); } +/* Phase 0 finding 2, pinned so a tree-sitter bump cannot change it quietly. + * + * The C grammar handles `_Thread_local` unevenly, and these are the three + * forms measured on the grammar shipped today: + * + * static _Thread_local int x = 0; parses clean + * static _Thread_local int *p; parses clean + * static _Thread_local char b[8]; fails — flagged as range 1-1 + * + * The array line really is missing from the graph, so flagging it is the + * honest answer, not a false positive. This test exists to make a grammar + * bump visible: if a newer grammar fixes the array form, this goes red and + * says so, instead of leaving a wrong note in the plan. (The plan's Phase 0 + * also listed the pointer form as failing. It does not fail today.) */ +TEST(c_thread_local_grammar_limit_is_pinned_issue963) { + CBMFileResult *ok = do_extract("static _Thread_local int x = 0;\n" + "void f(void) { x = 1; }\n", + CBM_LANG_C, "tls_init.c"); + ASSERT_NOT_NULL(ok); + ASSERT_FALSE(ok->parse_incomplete); + cbm_free_result(ok); + + CBMFileResult *ptr = do_extract("static _Thread_local int *p;\n" + "void f(void) { p = 0; }\n", + CBM_LANG_C, "tls_ptr.c"); + ASSERT_NOT_NULL(ptr); + ASSERT_FALSE(ptr->parse_incomplete); + cbm_free_result(ptr); + + CBMFileResult *arr = do_extract("static _Thread_local char b[8];\n" + "void f(void) { b[0] = 0; }\n", + CBM_LANG_C, "tls_arr.c"); + ASSERT_NOT_NULL(arr); + ASSERT_TRUE(arr->parse_incomplete); + ASSERT_NOT_NULL(arr->error_ranges); + /* The range names the one broken line, not the whole file. */ + ASSERT_STR_EQ("1-1", arr->error_ranges); + /* The clean function below it still reaches the graph. */ + ASSERT_TRUE(has_def(arr, "f")); + cbm_free_result(arr); + PASS(); +} + SUITE(parse_coverage) { RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete); RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted); @@ -752,4 +795,5 @@ SUITE(parse_coverage) { RUN_TEST(c_clean_file_stays_unflagged_after_refinement); RUN_TEST(perl_format_followed_by_named_sub_is_complete_issue1838); RUN_TEST(perl_malformed_source_remains_partial_issue1838); + RUN_TEST(c_thread_local_grammar_limit_is_pinned_issue963); } diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index d62a5572f..91d56183c 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -13061,6 +13061,87 @@ TEST(pipeline_markdown_and_config_prose_reaches_fts_body) { PASS(); } +#if defined(CBM_COVERAGE_MARKER_TEST_API) && CBM_COVERAGE_MARKER_TEST_API +/* Join two Studio Export range strings and hand back the result. The caller + * owns nothing: the string lives in the aggregate's arena, so copy it out + * before the arena goes away. */ +static void join_export_ranges(const char *agg_ranges, int agg_regions, const char *part_ranges, + int part_regions, char *out, size_t out_size, int *out_regions) { + CBMFileResult aggregate; + CBMFileResult part; + memset(&aggregate, 0, sizeof(aggregate)); + memset(&part, 0, sizeof(part)); + cbm_arena_init(&aggregate.arena); + cbm_arena_init(&part.arena); + aggregate.error_ranges = agg_ranges; + aggregate.error_region_count = agg_regions; + aggregate.parse_incomplete = true; + part.error_ranges = part_ranges; + part.error_region_count = part_regions; + part.parse_incomplete = true; + + out[0] = '\0'; + *out_regions = 0; + if (cbm_pipeline_coverage_marker_test_join(&aggregate, &part)) { + snprintf(out, out_size, "%s", aggregate.error_ranges ? aggregate.error_ranges : ""); + *out_regions = aggregate.error_region_count; + } + cbm_arena_destroy(&aggregate.arena); + cbm_arena_destroy(&part.arena); +} + +/* Count the "+" characters in a range string. A truncation marker must appear + * once and only at the end: every reader stops at the first token that is not + * a range, so a marker in the middle silently hides every range after it. */ +static int count_plus(const char *s) { + int n = 0; + for (const char *p = s; *p; p++) { + if (*p == '+') { + n++; + } + } + return n; +} + +TEST(pipeline_objectscript_export_range_join_keeps_one_trailing_marker) { + char joined[256]; + int regions = 0; + + /* Neither side dropped anything, so nothing invents a marker. */ + join_export_ranges("1-2,5-9", 2, "20-24", 1, joined, sizeof(joined), ®ions); + ASSERT_STR_EQ("1-2,5-9,20-24", joined); + ASSERT_EQ(0, count_plus(joined)); + ASSERT_EQ(3, regions); + + /* The first class overran the cap. Its marker must move to the end, so the + * second class's ranges stay visible in front of it. */ + join_export_ranges("1-2,5-9,+7", 2, "20-24", 1, joined, sizeof(joined), ®ions); + ASSERT_STR_EQ("1-2,5-9,20-24,+7", joined); + ASSERT_EQ(1, count_plus(joined)); + + /* The second class overran the cap. Same single trailing marker. */ + join_export_ranges("1-2", 1, "20-24,+3", 1, joined, sizeof(joined), ®ions); + ASSERT_STR_EQ("1-2,20-24,+3", joined); + ASSERT_EQ(1, count_plus(joined)); + + /* Both overran. One marker, carrying the sum, or the report would + * under-count what it threw away. */ + join_export_ranges("1-2,+7", 1, "20-24,+3", 1, joined, sizeof(joined), ®ions); + ASSERT_STR_EQ("1-2,20-24,+10", joined); + ASSERT_EQ(1, count_plus(joined)); + + /* An empty aggregate is the first class in the file. No leading comma. */ + join_export_ranges(NULL, 0, "20-24,+3", 1, joined, sizeof(joined), ®ions); + ASSERT_STR_EQ("20-24,+3", joined); + ASSERT_EQ(1, regions); + + /* A part with nothing to say leaves the aggregate exactly as it was. */ + join_export_ranges("1-2,+7", 1, "", 0, joined, sizeof(joined), ®ions); + ASSERT_STR_EQ("1-2,+7", joined); + PASS(); +} +#endif + SUITE(pipeline) { RUN_TEST(pipeline_lsp_surface_persisted_and_body_edit_invariant); /* Index lock */ @@ -13105,6 +13186,9 @@ SUITE(pipeline) { RUN_TEST(pipeline_objectscript_export_preserves_calls_sequential_parallel); RUN_TEST(pipeline_objectscript_export_incremental_matches_full_relationships); RUN_TEST(pipeline_objectscript_export_aggregate_exceeds_arena_block_table); +#if defined(CBM_COVERAGE_MARKER_TEST_API) && CBM_COVERAGE_MARKER_TEST_API + RUN_TEST(pipeline_objectscript_export_range_join_keeps_one_trailing_marker); +#endif RUN_TEST(pipeline_env_access_configures_sequential_parallel_parity); RUN_TEST(pipeline_call_reference_sequential_parallel_edge_set_parity); RUN_TEST(pipeline_incremental_cross_file_call_reference_matches_fresh_full); From 92bf67dba5a27993df169577d0c28f0d1aece024 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Mon, 31 Aug 2026 14:54:39 -0400 Subject: [PATCH 4/9] fix(coverage): name the whole-file number for what it is, a range end (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on this branch. Each parse_unusable entry carries one number, and the field was called "lines". That reads as the length of the file, and the two are not the same number: a grammar can end an error node past the last line, which this repo has already met — scripts/setup-windows.ps1 has 326 lines and its range ends at 327. A report whose whole thesis is honest reporting should not name that number after the wrong thing. "lines" also already means something else in this same response. Every search result carries a "lines" field holding a definition's line span. One word, two meanings, one document. The field is now "range_end", at both places that emit it — add_coverage_report reading the persisted rows, and add_parse_unusable_summary reading the per-run errors. The comment beside each one says the number can exceed the file, so the next reader does not have to rediscover it. Deriving the real file length instead was the other option and is not available here: neither cbm_file_error_t nor cbm_coverage_row_t carries it, only the path and the range string. One test, proved RED first — "range_end is NULL" against the old field name. It reads the end line from the persisted coverage row rather than a constant, so it states the property and not a measurement, and it asserts the old name is gone rather than kept beside the new one. index_resilience, parse_coverage and mcp: 283 passed, 4 skipped. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- src/mcp/mcp.c | 17 ++++--- tests/test_index_resilience.c | 89 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index ca78d8fa5..7fc30e8fe 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -4597,8 +4597,11 @@ static void add_coverage_report(yyjson_mut_doc *doc, yyjson_mut_val *root, cbm_s yyjson_mut_val *fe = yyjson_mut_obj(doc); yyjson_mut_obj_add_strcpy(doc, fe, "path", rows[i].rel_path); yyjson_mut_obj_add_bool(doc, fe, "whole_file", true); + /* The end of the range, not the length of the file. A grammar + * can end an error node past the last line, so this number can + * be larger than the file. See range_end_is_not_file_length. */ const char *dash = rows[i].detail ? strchr(rows[i].detail, '-') : NULL; - yyjson_mut_obj_add_int(doc, fe, "lines", dash ? atoi(dash + 1) : 0); + yyjson_mut_obj_add_int(doc, fe, "range_end", dash ? atoi(dash + 1) : 0); yyjson_mut_arr_add_val(pu_files, fe); } pu_n++; @@ -8019,13 +8022,13 @@ static void add_parse_partial_summary(yyjson_mut_doc *doc, yyjson_mut_val *root, /* Attach the whole-file half of the coverage summary. Always emits a top-level * "parse_unusable_count" (0 on clean runs) so the CI coverage gate can read it * without parsing anything else. When files were flagged: - * "parse_unusable": {"files":[{path,lines,whole_file}..(<=50)], "count":N, + * "parse_unusable": {"files":[{path,range_end,whole_file}..(<=50)], "count":N, * "truncated":bool, "note":"..."} * * These files WERE indexed, exactly like parse_partial ones. The difference is * that their range covers 80% or more of the file, so the range is not worth - * printing — "lines" gives the size and "whole_file" says plainly that reading - * the ranges is the same as reading the file. */ + * printing — "range_end" gives the last line the range names and "whole_file" + * says plainly that reading the ranges is the same as reading the file. */ static void add_parse_unusable_summary(yyjson_mut_doc *doc, yyjson_mut_val *root, const cbm_file_error_t *errs, int count) { int unusable = 0; @@ -8048,9 +8051,11 @@ static void add_parse_unusable_summary(yyjson_mut_doc *doc, yyjson_mut_val *root yyjson_mut_val *fe = yyjson_mut_obj(doc); yyjson_mut_obj_add_strcpy(doc, fe, "path", errs[i].path ? errs[i].path : ""); yyjson_mut_obj_add_bool(doc, fe, "whole_file", true); - /* The range string is "start-end"; its end line is the file length. */ + /* The end of the range, not the length of the file. A grammar can end + * an error node past the last line, so this number can be larger than + * the file. See range_end_is_not_file_length. */ const char *dash = errs[i].reason ? strchr(errs[i].reason, '-') : NULL; - yyjson_mut_obj_add_int(doc, fe, "lines", dash ? atoi(dash + 1) : 0); + yyjson_mut_obj_add_int(doc, fe, "range_end", dash ? atoi(dash + 1) : 0); yyjson_mut_arr_add_val(files, fe); shown++; } diff --git a/tests/test_index_resilience.c b/tests/test_index_resilience.c index 41bf29f2f..cefb9a592 100644 --- a/tests/test_index_resilience.c +++ b/tests/test_index_resilience.c @@ -488,6 +488,94 @@ TEST(index_parse_partial_reported) { PASS(); } +/* The whole-file class as index_status prints it, and what its number means. + * + * Each parse_unusable entry reports the END of the file's one range. The field + * was called "lines", which reads as the length of the file, and the two are + * not the same number — a grammar can end an error node past the last line, + * which this repo has already seen (a 326-line PowerShell file whose range + * ended at 327). "lines" also already means a definition's line span in the + * rest of this response, so the old name collided as well. + * + * The end line is checked against the persisted coverage row rather than a + * constant, so the test states the property and not a measurement. */ +TEST(index_parse_unusable_names_the_range_end) { + RProj lp; + memset(&lp, 0, sizeof(lp)); + snprintf(lp.tmpdir, sizeof(lp.tmpdir), "/tmp/cbm_resil_XXXXXX"); + if (!cbm_mkdtemp(lp.tmpdir)) { + FAIL("mkdtemp failed"); + } + rh_to_fwd_slashes(lp.tmpdir); + + /* Python gets no C preprocessor refinement, so a root-level ERROR still + * reports one range over the whole file — the parse_unusable class. */ + ri_write_text(lp.tmpdir, "unparseable.py", ")))\n((( \n]]] [[[\ndef x(:\n"); + ri_write_text(lp.tmpdir, "good.py", "def alpha():\n return 1\n"); + + char *resp = NULL; + cbm_store_t *store = ri_index_capture(&lp, &resp); + if (!resp) { + FAIL("no MCP response"); + } + if (!store) { + free(resp); + FAIL("store did not open"); + } + + /* The end line the report should be naming, read from the persisted row. */ + cbm_coverage_row_t *rows = NULL; + int cov_count = 0; + ASSERT_EQ(cbm_store_coverage_get(store, lp.project, &rows, &cov_count), CBM_STORE_OK); + int want_end = 0; + for (int i = 0; i < cov_count; i++) { + if (rows[i].rel_path && strstr(rows[i].rel_path, "unparseable.py") && rows[i].detail) { + const char *dash = strchr(rows[i].detail, '-'); + if (dash) { + want_end = atoi(dash + 1); + } + } + } + cbm_store_free_coverage(rows, cov_count); + ASSERT_GT(want_end, 0); + + yyjson_doc *d = yyjson_read(resp, strlen(resp), 0); + ASSERT_NOT_NULL(d); + yyjson_val *sc = yyjson_obj_get(yyjson_doc_get_root(d), "structuredContent"); + ASSERT_NOT_NULL(sc); + ASSERT_EQ(yyjson_get_int(yyjson_obj_get(sc, "parse_unusable_count")), 1); + + yyjson_val *pu = yyjson_obj_get(sc, "parse_unusable"); + ASSERT_NOT_NULL(pu); + yyjson_val *files = yyjson_obj_get(pu, "files"); + ASSERT_NOT_NULL(files); + int found = 0; + size_t idx = 0; + size_t fmax = 0; + yyjson_val *fe = NULL; + yyjson_arr_foreach(files, idx, fmax, fe) { + const char *fp = yyjson_get_str(yyjson_obj_get(fe, "path")); + /* The clean neighbour must not be listed at all. */ + ASSERT_NULL(fp ? strstr(fp, "good.py") : NULL); + if (!fp || !strstr(fp, "unparseable.py")) { + continue; + } + found = 1; + yyjson_val *range_end = yyjson_obj_get(fe, "range_end"); + ASSERT_NOT_NULL(range_end); + ASSERT_EQ(yyjson_get_int(range_end), want_end); + ASSERT_TRUE(yyjson_get_bool(yyjson_obj_get(fe, "whole_file"))); + /* The old name is gone, not kept beside the new one. */ + ASSERT_NULL(yyjson_obj_get(fe, "lines")); + } + ASSERT_TRUE(found); + + yyjson_doc_free(d); + free(resp); + rh_cleanup(&lp, store); + PASS(); +} + /* INV(parse-partial-clears-on-fix, #963): the persisted coverage signal must * stay FRESH — after the broken file is fixed and the project re-indexed * (incremental route: the DB already exists), its parse_partial row is gone @@ -812,6 +900,7 @@ SUITE(index_resilience) { RUN_TEST(index_clean_run_no_logfile); RUN_TEST(index_parse_partial_reported); RUN_TEST(index_parse_partial_clears_on_fix); + RUN_TEST(index_parse_unusable_names_the_range_end); RUN_TEST(index_not_indexed_by_design_reported); RUN_TEST(index_relative_repo_path_canonicalized); } From 41d78d3be9a413163f6f0a98315f55b264632c22 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 20:45:09 -0400 Subject: [PATCH 5/9] fix(coverage): stop reporting a duplicate range and a line past EOF (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/setup-windows.ps1 has 326 lines and its parse-coverage report read "113-113,113-113,245-327" — the same line named twice, and an end line that does not exist. Two separate faults, both in cbm_error_regions_push. Past-EOF end line. A tree-sitter node that ends at column 0 stopped right after the previous line's newline, so it holds no text on the row it points at. Adding 1 to that row named a line past the end of the file whenever the region ran to EOF. The node here is start=(244,2) end=(326,0). Clamp the end to the row above when the end column is 0 and the node spans more than one row. Duplicate range. Line 113 carries two separate ERROR nodes, at columns 25-29 and 31-32, and each pushed its own range. A line range says nothing new the second time. Drop a range that exactly repeats the one already open. The drop runs BEFORE the cap check, so a repeat is never miscounted as a range the cap threw away. Only an EXACT repeat is dropped, never a range that merely overlaps. Each range is judged separately afterwards by cbm_region_is_recovered, which asks whether definitions starting inside that range cover it. Two ranges holding the same numbers always get the same verdict, so dropping the repeat changes nothing. Two different ranges do not. Merging 3-3 into 2-3 hands the wider range's covering definition to an error that definition does not explain, and a real parse failure then vanishes from the report. That is not hypothetical. An earlier version of this commit merged on overlap and broke perl_malformed_source_remains_partial_issue1838, the test added with the Perl grammar refresh in 17b5a432. The malformed fixture produces two ERROR nodes, at lines 2-3 and 3-3. Merged, the 2-3 range looks fully covered by before_error and is removed, so parse_incomplete comes back false on a file that plainly does not parse. That test now pins this boundary. The real file reports "113-113,245-326". Two tests, both proved RED first with the exact expected text: coverage_repeated_error_line_reports_one_range_issue963 "2-2,2-2" != "2-2" coverage_range_never_ends_past_the_last_line_issue963 "1-5" != "1-4" Suites run on this change: parse_coverage 34, extraction 325, pipeline 264, mcp 246, index_resilience 7 — all passing. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- internal/cbm/cbm.c | 39 ++++++++++++++++++++++++++++-- tests/test_parse_coverage.c | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index c091561eb..01352a619 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -790,12 +790,47 @@ typedef struct { } cbm_error_regions_t; static void cbm_error_regions_push(cbm_error_regions_t *acc, TSNode n) { + TSPoint start = ts_node_start_point(n); + TSPoint end = ts_node_end_point(n); + uint32_t start_line = start.row + 1; + uint32_t end_line = end.row + 1; + + /* A node that ends at column 0 stopped right after the previous line's + * newline, so it holds no text on the row it points at. Counting that row + * named a line past the end of the file whenever the region ran to EOF: + * scripts/setup-windows.ps1 has 326 lines and reported "245-327". */ + if (end.column == 0 && end.row > start.row) { + end_line = end.row; + } + + /* One line can carry several error nodes, and repeating the same line range + * says nothing new. Line 113 of scripts/setup-windows.ps1 has two error + * nodes, at columns 25-29 and 31-32, and the report read "113-113,113-113". + * Drop the repeat. + * + * Only an EXACT repeat of the range already open is dropped. Do not merge + * ranges that merely overlap. Each range is judged separately later by + * cbm_region_is_recovered, which asks whether definitions starting inside + * that range cover it. Two ranges with the same numbers always get the same + * verdict, so collapsing them changes nothing. Two DIFFERENT ranges do not: + * merging 3-3 into 2-3 hands the wider range's covering definition to an + * error the definition does not explain, and a real parse failure then + * disappears from the report. tests/test_parse_coverage.c pins that case in + * perl_malformed_source_remains_partial_issue1838. + * + * This runs BEFORE the cap check, so a dropped repeat never counts as a + * range the cap threw away. */ + if (acc->count > 0 && start_line == acc->starts[acc->count - 1] && + end_line == acc->ends[acc->count - 1]) { + return; + } + if (acc->count >= CBM_MAX_ERROR_REGIONS) { acc->dropped++; return; } - acc->starts[acc->count] = ts_node_start_point(n).row + 1; - acc->ends[acc->count] = ts_node_end_point(n).row + 1; + acc->starts[acc->count] = start_line; + acc->ends[acc->count] = end_line; acc->count++; } diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index 695d41992..3e00958da 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -763,6 +763,51 @@ TEST(c_thread_local_grammar_limit_is_pinned_issue963) { PASS(); } +/* Two error nodes can sit on ONE line. Line 113 of scripts/setup-windows.ps1 + * does exactly that, and the report used to read "113-113,113-113" — the same + * line named twice. A line range says nothing new the second time, so repeated + * or overlapping regions must collapse into one. */ +static const char *PS_TWO_ERRORS_ONE_LINE = "Write-Host \"start\"\n" /* 1 */ + "wsl.exe -- bash -c $Command 2>&1\n" /* 2 */ + "Write-Host \"end\"\n"; /* 3 */ + +/* An error region that runs to the end of the file stops just after the last + * newline. Tree-sitter calls that position row N, column 0 — a row that holds + * no text. Reading it as a line number named a line past the end of the file: + * scripts/setup-windows.ps1 has 326 lines and the report said "245-327". */ +static const char *PS_ERROR_TO_EOF = "} else {\n" /* 1 */ + " if ($a) {\n" /* 2 */ + " Write-Host x\n" /* 3 */ + "}\n"; /* 4 */ + +TEST(coverage_repeated_error_line_reports_one_range_issue963) { + CBMFileResult *r = + cbm_extract_file(PS_TWO_ERRORS_ONE_LINE, (int)strlen(PS_TWO_ERRORS_ONE_LINE), + CBM_LANG_POWERSHELL, "covproj", "two_errors.ps1", 0, NULL, NULL); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_NOT_NULL(r->error_ranges); + /* Line 2 carries two separate error nodes. It must be named once. */ + ASSERT_STR_EQ(r->error_ranges, "2-2"); + ASSERT_EQ(r->error_region_count, 1); + cbm_free_result(r); + PASS(); +} + +TEST(coverage_range_never_ends_past_the_last_line_issue963) { + int len = (int)strlen(PS_ERROR_TO_EOF); + CBMFileResult *r = cbm_extract_file(PS_ERROR_TO_EOF, len, CBM_LANG_POWERSHELL, "covproj", + "error_to_eof.ps1", 0, NULL, NULL); + ASSERT_NOT_NULL(r); + ASSERT_TRUE(r->parse_incomplete); + ASSERT_NOT_NULL(r->error_ranges); + /* The file has four lines and ends with a newline. Line 5 does not exist. */ + ASSERT_STR_EQ(r->error_ranges, "1-4"); + ASSERT_NULL(strstr(r->error_ranges, "5")); + cbm_free_result(r); + PASS(); +} + SUITE(parse_coverage) { RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete); RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted); @@ -796,4 +841,6 @@ SUITE(parse_coverage) { RUN_TEST(perl_format_followed_by_named_sub_is_complete_issue1838); RUN_TEST(perl_malformed_source_remains_partial_issue1838); RUN_TEST(c_thread_local_grammar_limit_is_pinned_issue963); + RUN_TEST(coverage_repeated_error_line_reports_one_range_issue963); + RUN_TEST(coverage_range_never_ends_past_the_last_line_issue963); } From ac0ce605e50b0ad7fd546bff11f76d3af429a40d Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 00:39:43 -0400 Subject: [PATCH 6/9] ci(coverage): fail a PR when this repo's own parse-coverage report goes bad (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A coverage range is advice — "these lines are missing from the graph, read them". It stops being advice when it names most of the file, and it stops being honest when the list was clipped without saying so. Both happened here: src/cli/cli.c reported its whole 13,046 lines as one range, and two caps in series dropped ranges with no signal. Nothing would have caught either. scripts/ci/self-index-coverage-gate.sh indexes this repo with the binary just built and fails on any of four things: 1. A file reports a whole-file parse failure (parse_unusable). Zero today. 2. Any range string carries the "+" truncation marker. With the cap at 256, a file that still overflows is worth stopping for. 3. Any single range covers more than 25% of its file, for files of 200 lines or more. The floor matters: a 5-line PL/SQL limitation fixture with a 3-line range is 60% of itself and says nothing about report quality. 4. parse_partial_count rises above the ceiling in parse-partial-baseline.txt (58 today). This complements the FLOOR in tests/test_index_resilience.c, which stops the signal being switched off by accident. Every check was verified to FAIL, not just to pass: empty allowlist -> setup-windows.ps1 flagged at 25.5% MAX_SINGLE_RANGE_PCT=3 -> cli.c flagged at 3.9% ceiling 57 -> parse_partial_count 58 flagged a repo of broken files -> 4 whole-file failures flagged a 1200-line garbage file -> its clipped range list flagged scripts/setup-windows.ps1 is the one allowlist entry, and it is a real gap rather than noise: one range covers lines 245-327 of a 326-line file because the tree-sitter PowerShell grammar cannot parse the `} else {` branch running to EOF, so those 83 lines genuinely are absent from the graph. Every other file of 200+ lines sits at 3.9% or below, so the 25% threshold has room and should not be raised to hide this. Wired into the existing pr-smoke job, Ubuntu leg only. That job is already in ci-ok's needs, so the gate is a required check with no workflow-graph surgery. Ubuntu only because the flagged ranges depend on which conditional-compilation branches the preprocessor keeps — on a machine where _WIN32 is defined a different set of lines is flagged, which is why the gate asserts proportions and never exact line numbers. The changes filter now notices edits to the gate, the allowlist and the baseline. Runs in 21 seconds. Not extended into scripts/smoke-invariants.sh on purpose: that runs from smoke.yml, whose triggers are workflow_dispatch and push to qa/smoke-**, and which is documented non-gating — it would never run on a PR. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- .github/workflows/pr.yml | 11 ++- scripts/ci/coverage-gate-allowlist.txt | 15 ++++ scripts/ci/parse-partial-baseline.txt | 7 ++ scripts/ci/self-index-coverage-gate.sh | 118 +++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 scripts/ci/coverage-gate-allowlist.txt create mode 100644 scripts/ci/parse-partial-baseline.txt create mode 100755 scripts/ci/self-index-coverage-gate.sh diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index c18a45d13..8d854b06c 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -59,7 +59,7 @@ jobs: # The paginated files endpoint remains filename-only for this gate. FILES=$(gh api --paginate "repos/$REPO/pulls/$PR/files?per_page=100" --jq '.[].filename') printf '%s\n' "$FILES" - if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then + if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|scripts/ci/(self-index-coverage-gate\.sh|coverage-gate-allowlist\.txt|parse-partial-baseline\.txt)|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then echo "product=true" >> "$GITHUB_OUTPUT" else echo "product=false" >> "$GITHUB_OUTPUT" @@ -127,6 +127,15 @@ jobs: CCACHE_DIR: ${{ github.workspace }}/.ccache CCACHE_MAXSIZE: 1000M + # Index this repo with the binary just built and check its own + # parse-coverage report is still useful advice (#963). Ubuntu only: the + # flagged line ranges depend on which conditional-compilation branches + # the preprocessor keeps, so they differ per platform. The gate asserts + # proportions, never exact line numbers. + - name: Parse-coverage gate (Ubuntu) + if: matrix.os == 'ubuntu-latest' + run: scripts/ci/self-index-coverage-gate.sh "$(pwd)/build/c/codebase-memory-mcp" + - name: Build prod + smoke (macOS) if: matrix.os == 'macos-14' run: | diff --git a/scripts/ci/coverage-gate-allowlist.txt b/scripts/ci/coverage-gate-allowlist.txt new file mode 100644 index 000000000..eb5159993 --- /dev/null +++ b/scripts/ci/coverage-gate-allowlist.txt @@ -0,0 +1,15 @@ +# Files the self-index coverage gate skips, one repo-relative path per line. +# +# Adding a line here is a deliberate decision, not a convenience. It says: +# "we know this file reports a wide parse-coverage range, we have looked at +# why, and we accept it." Write the reason above the path. A line with no +# reason should be removed rather than trusted. +# +# Blank lines and lines starting with # are ignored. + +# One range covers lines 245-327 of a 326-line file (25.5%). The tree-sitter +# PowerShell grammar cannot parse the `} else {` branch that runs to the end +# of the file, so those 83 lines really are absent from the graph. This is a +# genuine grammar gap, not a reporting error. Every other file of 200+ lines +# in this repo sits at 3.9% or below. +scripts/setup-windows.ps1 diff --git a/scripts/ci/parse-partial-baseline.txt b/scripts/ci/parse-partial-baseline.txt new file mode 100644 index 000000000..0803effe1 --- /dev/null +++ b/scripts/ci/parse-partial-baseline.txt @@ -0,0 +1,7 @@ +# Ceiling for parse_partial_count when this repo indexes itself. +# +# The number below is what the gate allows. It complements the FLOOR asserted +# in tests/test_index_resilience.c, which stops the signal being switched off +# by accident. Raising this number is allowed but should be explained in the +# commit that does it. +58 diff --git a/scripts/ci/self-index-coverage-gate.sh b/scripts/ci/self-index-coverage-gate.sh new file mode 100755 index 000000000..e2eebca7e --- /dev/null +++ b/scripts/ci/self-index-coverage-gate.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# Regression guard: this repo's own parse-coverage report must stay useful. +# +# A coverage range is advice — "these lines are missing from the graph, read +# them". Advice stops being advice when it names most of the file, and it +# stops being honest when the list was clipped without saying so. Both things +# happened here before (#963): src/cli/cli.c reported its whole 13,046 lines +# as one range, and two caps in series dropped ranges with no signal at all. +# +# This indexes the repo with a given binary and fails if any of that comes back. +# +# Usage: self-index-coverage-gate.sh +# +# NOTE ON PLATFORM: the ranges depend on which conditional-compilation branches +# the preprocessor keeps. On a machine where _WIN32 is defined the discarded +# branches swap and a different set of lines is flagged. That is why this runs +# on ONE CI leg and asserts proportions rather than exact line numbers. +set -euo pipefail + +BIN="${1:?usage: self-index-coverage-gate.sh }" +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +ALLOWLIST="${REPO_ROOT}/scripts/ci/coverage-gate-allowlist.txt" +BASELINE_FILE="${REPO_ROOT}/scripts/ci/parse-partial-baseline.txt" + +# Share of a file one range may cover before it stops being useful advice. +# The worst real offender today is src/cli/cli.c at 3.9%, so this has room. +MAX_SINGLE_RANGE_PCT="${MAX_SINGLE_RANGE_PCT:-25}" +# Files below this are exempt: a 5-line fixture with a 3-line range is 60% of +# itself and says nothing about report quality. +MIN_FILE_LINES="${MIN_FILE_LINES:-200}" + +command -v jq >/dev/null || { echo "FAIL: jq is required"; exit 1; } + +WORK="$(mktemp -d)" +# The runtime dir holds a unix socket, and a socket path has a hard length +# limit (~104 bytes). macOS puts mktemp under /var/folders//T/, which +# blows that limit and fails with "secure CLI coordination could not be +# created (endpoint)". Keep the runtime dir short and separate from the cache. +RUNTIME="/tmp/cbm-gate.$$" +trap 'rm -rf "$WORK" "$RUNTIME"' EXIT +export CBM_CACHE_DIR="${WORK}/cache" +export CBM_RUNTIME_DIR="$RUNTIME" +mkdir -p "$CBM_CACHE_DIR" "$RUNTIME" + +echo "==> indexing ${REPO_ROOT} with $(basename "$BIN")" +"$BIN" cli index_repository --repo-path "$REPO_ROOT" --mode full --json \ + > "${WORK}/index.json" 2>"${WORK}/index.err" || { + echo "FAIL: index_repository exited non-zero"; tail -20 "${WORK}/index.err"; exit 1; } + +PROJECT="$(jq -r '.structuredContent.project // empty' "${WORK}/index.json")" +[ -n "$PROJECT" ] || { echo "FAIL: index_repository did not name a project"; exit 1; } + +"$BIN" cli index_status --project "$PROJECT" --json > "${WORK}/status.json" 2>/dev/null || { + echo "FAIL: index_status exited non-zero"; exit 1; } + +# Allowlisted paths, comments and blanks stripped. +ALLOWED="${WORK}/allowed.txt" +: > "$ALLOWED" +[ -f "$ALLOWLIST" ] && sed -e 's/#.*//' -e 's/[[:space:]]*$//' "$ALLOWLIST" \ + | grep -v '^$' > "$ALLOWED" || true + +FAILURES=0 +note_failure() { echo "FAIL: $*"; FAILURES=$((FAILURES + 1)); } + +# ── 1. Nothing may fail across a whole file ──────────────────────────────── +# parse_unusable means one range covers 80%+ of the file, so the report tells +# a reader to go read the source. Zero today; a new one is a real regression. +UNUSABLE="$(jq -r '.structuredContent.parse_unusable.count // 0' "${WORK}/status.json")" +UNUSABLE_LISTED="$(jq -r '[.structuredContent.parse_unusable.files[]?.path]|join(" ")' \ + "${WORK}/status.json")" +for p in $UNUSABLE_LISTED; do + grep -qxF "$p" "$ALLOWED" && UNUSABLE=$((UNUSABLE - 1)) +done +if [ "$UNUSABLE" -gt 0 ]; then + note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_LISTED}" +fi + +# ── 2. No range list may be silently clipped ────────────────────────────── +# A trailing "+" says the producer's cap threw N ranges away. With the cap +# at 256 a file that still overflows is worth stopping for. +TRUNCATED="$(jq -r '[.structuredContent.parse_partial.files[]? + | select(.error_ranges? // "" | test("\\+[0-9]+$")) | .path] | join(" ")' \ + "${WORK}/status.json")" +for p in $TRUNCATED; do + grep -qxF "$p" "$ALLOWED" && continue + note_failure "$p carries a +N truncation marker — its range list was clipped" +done + +# ── 3. No single range may cover a quarter of its file ──────────────────── +while IFS=$'\t' read -r path ranges; do + [ -n "$path" ] || continue + grep -qxF "$path" "$ALLOWED" && continue + [ -f "${REPO_ROOT}/${path}" ] || continue + total="$(wc -l < "${REPO_ROOT}/${path}" | tr -d ' ')" + [ "$total" -ge "$MIN_FILE_LINES" ] || continue + widest="$(printf '%s' "$ranges" | tr ',' '\n' | grep '^[0-9]' \ + | awk -F- '{d=$2-$1+1; if (d>m) m=d} END {print m+0}')" + pct="$(awk -v a="$widest" -v b="$total" 'BEGIN{printf "%.1f", 100*a/b}')" + over="$(awk -v p="$pct" -v lim="$MAX_SINGLE_RANGE_PCT" 'BEGIN{print (p>lim)?1:0}')" + if [ "$over" = "1" ]; then + note_failure "$path has one range of ${widest} lines — ${pct}% of ${total}, over ${MAX_SINGLE_RANGE_PCT}%" + fi +done < <(jq -r '.structuredContent.parse_partial.files[]? + | "\(.path)\t\(.error_ranges // "")"' "${WORK}/status.json") + +# ── 4. The flagged-file count must not drift upward unnoticed ───────────── +CEILING="$(sed -e 's/#.*//' "$BASELINE_FILE" | grep -oE '[0-9]+' | head -1)" +PARTIAL="$(jq -r '.structuredContent.parse_partial.count // 0' "${WORK}/status.json")" +if [ "$PARTIAL" -gt "$CEILING" ]; then + note_failure "parse_partial_count is ${PARTIAL}, above the ceiling ${CEILING} in $(basename "$BASELINE_FILE")" +fi + +echo "==> parse_partial=${PARTIAL} (ceiling ${CEILING}) parse_unusable=${UNUSABLE} allowlisted=$(wc -l < "$ALLOWED" | tr -d ' ')" +if [ "$FAILURES" -gt 0 ]; then + echo "FAIL: ${FAILURES} coverage-gate check(s) failed" + exit 1 +fi +echo "PASS: parse-coverage report is within bounds" From 988f1b5b006123f65c04145b772bdf1cf0e0c1f2 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 20:45:25 -0400 Subject: [PATCH 7/9] ci(coverage): fix three ways the coverage gate could pass without checking (#963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verifying the gate against a locally built binary turned up three defects in the gate itself, all of the same shape it exists to catch: it could report PASS without having looked at everything. Reading the ceiling aborted the whole gate on a long file. The ceiling came from `sed | grep -oE | head -1`. Under `set -o pipefail` head closes the pipe, grep dies of SIGPIPE, and the gate exits with no message at all. It does not bite on today's 7-line baseline file and it does bite on a long one, which is proved. Replaced with one awk that stops after the first number. The report's own truncation flag was never read. index_status lists at most 500 files per class (COVERAGE_FILE_CAP) and sets "truncated" when it dropped the rest. Checks 2 and 3 walk that list file by file, so a clipped list means they judge part of the repo and still print PASS. New check 0 stops instead. Today the list holds all 58 files and truncated is false, so this is a guard, not a fix for live behaviour. Check 1 named files it had already accepted. It subtracted allowlisted paths from the count but still printed them in the failure text, so the message disagreed with the number beside it. It now counts and names the same set. The gate also did not answer --help, which scripts/ci/README.md says every script there does. It fed --help to basename, printed a usage error from the wrong program and exited 0. It now prints a Usage: block and exits 0, rejects an unknown flag with exit 2 and the house line "Please consult --help.", and is enrolled in HELP_ENTRIES and STRICT_ENTRIES in the venue parity contract so the rule is enforced rather than only written down. Breaking --help fails that contract with exit 1, which is checked. Added the missing row to the scripts/ci/README.md table. The allowlist reason for scripts/setup-windows.ps1 carried the old numbers. After the range fix in the previous commit the file reports 113-113,245-326, so the widest range is 82 lines rather than 83 — 25.2% of 326. Still over the 25% limit, so the entry stays, and the reason now says why narrowing it by one line did not clear the gate. Verified against a locally built binary: healthy PASS, parse_partial=58 (ceiling 58) parse_unusable=0, exit 0 allowlist emptied FAIL naming setup-windows.ps1 at 25.2%, exit 1 ceiling at 57 FAIL naming the count, exit 1 both restored PASS, exit 0 Venue parity contract: 19 --help entries, 9 strict-flag entries, green. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- scripts/ci/README.md | 1 + scripts/ci/coverage-gate-allowlist.txt | 8 ++- scripts/ci/self-index-coverage-gate.sh | 75 ++++++++++++++++++++++++-- tests/test_venue_parity_contract.sh | 2 + 4 files changed, 79 insertions(+), 7 deletions(-) diff --git a/scripts/ci/README.md b/scripts/ci/README.md index f296e6fc5..a6cab8f5d 100644 --- a/scripts/ci/README.md +++ b/scripts/ci/README.md @@ -13,6 +13,7 @@ CI and the local infrastructure — both of which the venue-parity contract | `preflight-docker.sh` | Same idea for Colima/docker: prune runner-unlike residue, assert free space on the filesystem backing the docker data root (not the VM's `/`). Build cache + named volumes KEPT (the local analogue of actions/cache); `--deep` drops them. | `test-infrastructure/run.sh` | | `check-glibc-compat.sh` | Run a linux binary in debian:bullseye (glibc 2.31) — the portable binary must start on old glibc. | `_smoke.yml` portable legs | | `generate-sbom.py` | The release SPDX SBOM (vendored versions reviewable here, diffable by vendoring PRs — was inline YAML). | `release.yml` | +| `self-index-coverage-gate.sh` | Index this repo with the binary just built and fail the PR if its own parse-coverage report stops being useful advice: no whole-file failure, no range list clipped without saying so, no single range over 25% of a file of 200+ lines, and the flagged-file count at or below the checked-in ceiling. Reads its two data files, `coverage-gate-allowlist.txt` (paths it skips, each with a written reason) and `parse-partial-baseline.txt` (the ceiling). Ubuntu leg only — the flagged lines depend on which conditional-compilation branches the preprocessor keeps. | `pr.yml pr-smoke` | | `require-all-green.sh` | The aggregate gate: fail unless every needed job succeeded or legitimately skipped (was inline YAML). | `pr.yml ci-ok` | | `verify-shard-union.sh` | Prove sharded test legs lost nothing: shard count agreement, indices 1..n, identical suite lists, union of slices == full list (was inline YAML). | `_test.yml` shard-completeness | | `prepare-release-candidates.sh` | Copy one linker output into stripped/unstripped candidates, finalize signatures, composition-check them without execution, and record their hashes. | `_build.yml`, local artifact smoke | diff --git a/scripts/ci/coverage-gate-allowlist.txt b/scripts/ci/coverage-gate-allowlist.txt index eb5159993..697cae590 100644 --- a/scripts/ci/coverage-gate-allowlist.txt +++ b/scripts/ci/coverage-gate-allowlist.txt @@ -7,9 +7,13 @@ # # Blank lines and lines starting with # are ignored. -# One range covers lines 245-327 of a 326-line file (25.5%). The tree-sitter +# One range covers lines 245-326 of a 326-line file (25.2%). The tree-sitter # PowerShell grammar cannot parse the `} else {` branch that runs to the end -# of the file, so those 83 lines really are absent from the graph. This is a +# of the file, so those 82 lines really are absent from the graph. This is a # genuine grammar gap, not a reporting error. Every other file of 200+ lines # in this repo sits at 3.9% or below. +# +# The range read 113-113,113-113,245-327 until the duplicate and the past-EOF +# end line were fixed in cbm_error_regions_push. Narrowing it by one line did +# NOT clear the gate: 25.2% is still over the 25% limit, so this entry stays. scripts/setup-windows.ps1 diff --git a/scripts/ci/self-index-coverage-gate.sh b/scripts/ci/self-index-coverage-gate.sh index e2eebca7e..00662a2b6 100755 --- a/scripts/ci/self-index-coverage-gate.sh +++ b/scripts/ci/self-index-coverage-gate.sh @@ -17,7 +17,52 @@ # on ONE CI leg and asserts proportions rather than exact line numbers. set -euo pipefail -BIN="${1:?usage: self-index-coverage-gate.sh }" +usage() { + cat <<'EOF' +Usage: scripts/ci/self-index-coverage-gate.sh + +Index THIS repository with the given binary and fail if the repository's own +parse-coverage report stops being useful advice (#963). + +Four checks, all read from index_status: + 1. No file reports a whole-file parse failure (parse_unusable). + 2. No range list was clipped without saying so (a trailing "+" marker). + 3. No single range covers more than 25% of a file of 200 lines or more. + 4. The flagged-file count stays at or below the checked-in ceiling. + +Data files, both beside this script: + coverage-gate-allowlist.txt paths the checks skip, each with a written + reason above it. + parse-partial-baseline.txt the ceiling for check 4. + +Environment: + MAX_SINGLE_RANGE_PCT Share limit for check 3 (default 25). + MIN_FILE_LINES Files below this are exempt from check 3 (default 200). + +Exit 0 when every check passes, 1 when any fails, 2 on a bad argument. + +Options: + -h, --help This text. +EOF +} + +BIN="" +while [ $# -gt 0 ]; do + case "$1" in + -h | --help) usage; exit 0 ;; + -*) echo "self-index-coverage-gate: unknown argument '$1'. Please consult --help." >&2; exit 2 ;; + *) + [ -z "$BIN" ] || { + echo "self-index-coverage-gate: one binary path only. Please consult --help." >&2 + exit 2 + } + BIN="$1" + ;; + esac + shift +done +[ -n "$BIN" ] || { echo "self-index-coverage-gate: need a binary path. Please consult --help." >&2; exit 2; } + REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" ALLOWLIST="${REPO_ROOT}/scripts/ci/coverage-gate-allowlist.txt" BASELINE_FILE="${REPO_ROOT}/scripts/ci/parse-partial-baseline.txt" @@ -62,17 +107,32 @@ ALLOWED="${WORK}/allowed.txt" FAILURES=0 note_failure() { echo "FAIL: $*"; FAILURES=$((FAILURES + 1)); } +# ── 0. The report's own file list must be complete ──────────────────────── +# index_status lists at most 500 files per class and sets "truncated" when it +# drops the rest. Checks 2 and 3 below read that list file by file, so a +# clipped list means they judge only part of the repo and still print PASS. +# That is the same silent clipping this gate exists to catch, so stop instead. +for cls in parse_partial parse_unusable; do + clipped="$(jq -r --arg c "$cls" '.structuredContent[$c].truncated // false' "${WORK}/status.json")" + if [ "$clipped" = "true" ]; then + note_failure "index_status clipped its ${cls} file list — the checks below would see only part of it" + fi +done + # ── 1. Nothing may fail across a whole file ──────────────────────────────── # parse_unusable means one range covers 80%+ of the file, so the report tells # a reader to go read the source. Zero today; a new one is a real regression. -UNUSABLE="$(jq -r '.structuredContent.parse_unusable.count // 0' "${WORK}/status.json")" UNUSABLE_LISTED="$(jq -r '[.structuredContent.parse_unusable.files[]?.path]|join(" ")' \ "${WORK}/status.json")" +UNUSABLE=0 +UNUSABLE_KEPT="" for p in $UNUSABLE_LISTED; do - grep -qxF "$p" "$ALLOWED" && UNUSABLE=$((UNUSABLE - 1)) + grep -qxF "$p" "$ALLOWED" && continue + UNUSABLE=$((UNUSABLE + 1)) + UNUSABLE_KEPT="${UNUSABLE_KEPT}${p} " done if [ "$UNUSABLE" -gt 0 ]; then - note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_LISTED}" + note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_KEPT}" fi # ── 2. No range list may be silently clipped ────────────────────────────── @@ -104,7 +164,12 @@ done < <(jq -r '.structuredContent.parse_partial.files[]? | "\(.path)\t\(.error_ranges // "")"' "${WORK}/status.json") # ── 4. The flagged-file count must not drift upward unnoticed ───────────── -CEILING="$(sed -e 's/#.*//' "$BASELINE_FILE" | grep -oE '[0-9]+' | head -1)" +# One awk, not `grep | head -1`: under `set -o pipefail` head closes the pipe +# early, grep dies of SIGPIPE, and the whole gate aborts with no message. It +# does not bite on today's short file, and it would bite the day someone adds +# a few comment lines. +CEILING="$(awk '{ sub(/#.*/, "") } match($0, /[0-9]+/) { print substr($0, RSTART, RLENGTH); exit }' \ + "$BASELINE_FILE")" PARTIAL="$(jq -r '.structuredContent.parse_partial.count // 0' "${WORK}/status.json")" if [ "$PARTIAL" -gt "$CEILING" ]; then note_failure "parse_partial_count is ${PARTIAL}, above the ceiling ${CEILING} in $(basename "$BASELINE_FILE")" diff --git a/tests/test_venue_parity_contract.sh b/tests/test_venue_parity_contract.sh index 4bcd694d7..001e5f856 100755 --- a/tests/test_venue_parity_contract.sh +++ b/tests/test_venue_parity_contract.sh @@ -388,6 +388,7 @@ scripts/smoke-invariants.sh scripts/ci/preflight-docker.sh scripts/ci/require-all-green.sh scripts/ci/verify-shard-union.sh +scripts/ci/self-index-coverage-gate.sh scripts/ci/generate-sbom.py scripts/package-release.sh scripts/ci/smoke-artifact.sh @@ -426,6 +427,7 @@ scripts/lint.sh scripts/smoke-local.sh scripts/soak-legs.sh scripts/ci/preflight-docker.sh +scripts/ci/self-index-coverage-gate.sh test-infrastructure/vm/vm-smoke.sh scripts/smoke-invariants.sh " From bf73da1b8118b0ae536d5e0388d97dafbe0f6c0b Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Mon, 31 Aug 2026 15:52:28 -0400 Subject: [PATCH 8/9] ci(coverage): raise the parse-partial ceiling to 59, for a gap main added (#963) The gate's check 4 failed on its own pull request: parse_partial_count is 59 against a ceiling of 58. The rise did not come from this branch. Measured with the binary built from this branch, three trees indexed: this branch, no merge 58 matches the baseline as written origin/main alone 59 main merged into it 59 identical file list to main alone CI tests the merge of a pull request into main, so the gate sees 59. The one file main added to the flagged list is src/daemon/runtime.c, at one line: src/daemon/runtime.c 47-47 1 line of 3291 0.03% of the file Line 47 is a function-style _Atomic declaration: static _Atomic(cbm_daemon_runtime_containment_hook_t) runtime_containment_hook_seam; The tree-sitter C grammar does not parse that form. The keyword form four lines above it, `static _Atomic uint32_t ...`, parses fine. This is the same class of grammar limitation this branch already pins for _Thread_local in tests/test_parse_coverage.c. It arrived with fc1b1ee7 on main. So the ceiling moves to 59 rather than the file being allowlisted. An allowlist entry is for a file whose single range is over the 25% limit and has a written reason to stay; one line out of 3291 is nowhere near it, and hiding the file would remove a real gap from the count the ceiling exists to watch. Known cost, worth stating plainly: a ceiling checked against a moving main drifts. Any merge to main that adds a partially-parsing file reddens every open pull request until someone edits this file by hand, and the pull request that goes red is never the one that caused it. This commit does not fix that - the fix is to compare against the merge base instead of a checked-in number, which changes what the gate is and belongs to whoever owns CI policy here. Filed separately. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Joshua Richter --- scripts/ci/parse-partial-baseline.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/ci/parse-partial-baseline.txt b/scripts/ci/parse-partial-baseline.txt index 0803effe1..b5d22da69 100644 --- a/scripts/ci/parse-partial-baseline.txt +++ b/scripts/ci/parse-partial-baseline.txt @@ -4,4 +4,8 @@ # in tests/test_index_resilience.c, which stops the signal being switched off # by accident. Raising this number is allowed but should be explained in the # commit that does it. -58 +# +# 58 -> 59 on 2026-08-31. main gained src/daemon/runtime.c, whose line 47 the +# tree-sitter C grammar cannot parse. The rise came from main, not from a +# branch. See the commit for the measurement. +59 From a3e8fa3d08691f050f16d75879fd8a84b73ad76a Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Tue, 1 Sep 2026 20:15:26 -0400 Subject: [PATCH 9/9] ci(coverage): compare the parse-coverage gate against the merge base (#963) The gate failed a pull request on the state of the report, not on the change the pull request made. Check 4 compared parse_partial_count against a number checked into scripts/ci/parse-partial-baseline.txt, and checks 1-3 asserted zero findings outright. Any of the four could go red for something main did. That is not theoretical. #1972 was this exact thing: main gained src/daemon/runtime.c, the count went 58 -> 59 on its own, and the number had to be raised by hand. #1824 will do it again and larger. Blazor .razor files map to C#, their markup lands in ERROR regions by design, and the count rises by roughly the repo's .razor count. A coverage improvement would read as a gate failure on an unrelated branch, and the person who hit it would have no way to tell that from a real regression. The gate now resolves the base commit, checks it out into a temporary worktree, and indexes both trees with the same binary. All four checks compare the two: 1. a whole-file parse failure fails only when it is new at head 2. a "+N" clipping marker fails only when it is new at head 3. a range over 25% of its file fails only when the file was within the share at the base 4. the flagged-file count fails only when it is above the base's parse-partial-baseline.txt stops being a gate. The script still prints the recorded number so a reader can see the drift, and says plainly that nothing fails on it. Nobody has to raise that number again. What this cannot see: both trees are indexed with the same binary, so a branch that changes the extractor itself moves the base side and the head side together and this gate will not fail on it. Catching that needs the base commit's own binary, which means a second full build -- about twelve minutes against the twenty-six seconds the whole gate step takes. Two things still cover it: the FLOOR asserted in tests/test_index_resilience.c stops the signal being switched off, and the absolute counts for both sides now print on every run, so a jump is visible in the log even when it does not fail. The script header and the scripts/ci/README.md row both say so. tests/test_coverage_gate_contract.sh pins the behaviour. It drives the production script with a fake binary that prints canned JSON, so no seam is added to the script itself and no indexing happens. Fifteen cases: each of the four findings present at both sides (pass) and new at head (fail), the count equal to, below and above the base, the recorded number not gating, both sides printing, a clipped file list still stopping the run outright, the allowlist skipping a path, and an unresolvable base commit stopping the run. Verified by reverting each of the four comparisons one at a time and confirming the matching "present at both sides" case goes red, then restoring. A real run against a built binary passes with both sides reported, and takes 45s for two indexes. pr.yml passes COVERAGE_GATE_BASE_SHA so the gate uses the commit GitHub itself used to build the merge, rather than falling back to the first parent of HEAD. Refs #963, #1972 Signed-off-by: Joshua Richter --- .github/workflows/pr.yml | 8 +- scripts/ci/README.md | 2 +- scripts/ci/parse-partial-baseline.txt | 21 +- scripts/ci/self-index-coverage-gate.sh | 275 +++++++++++++++++-------- scripts/test.sh | 3 + tests/test_coverage_gate_contract.sh | 241 ++++++++++++++++++++++ 6 files changed, 456 insertions(+), 94 deletions(-) create mode 100755 tests/test_coverage_gate_contract.sh diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 8d854b06c..2db1e0d31 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -59,7 +59,7 @@ jobs: # The paginated files endpoint remains filename-only for this gate. FILES=$(gh api --paginate "repos/$REPO/pulls/$PR/files?per_page=100" --jq '.[].filename') printf '%s\n' "$FILES" - if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|scripts/ci/(self-index-coverage-gate\.sh|coverage-gate-allowlist\.txt|parse-partial-baseline\.txt)|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then + if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|scripts/ci/(self-index-coverage-gate\.sh|coverage-gate-allowlist\.txt|parse-partial-baseline\.txt)|tests/test_coverage_gate_contract\.sh|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then echo "product=true" >> "$GITHUB_OUTPUT" else echo "product=false" >> "$GITHUB_OUTPUT" @@ -134,6 +134,12 @@ jobs: # proportions, never exact line numbers. - name: Parse-coverage gate (Ubuntu) if: matrix.os == 'ubuntu-latest' + env: + # The gate compares this branch against the merge base, so it needs + # the commit GitHub used to build the merge. Without this it falls + # back to the first parent of HEAD, which is the same commit on a + # pull_request checkout — this makes it explicit rather than lucky. + COVERAGE_GATE_BASE_SHA: ${{ github.event.pull_request.base.sha }} run: scripts/ci/self-index-coverage-gate.sh "$(pwd)/build/c/codebase-memory-mcp" - name: Build prod + smoke (macOS) diff --git a/scripts/ci/README.md b/scripts/ci/README.md index a6cab8f5d..f14549451 100644 --- a/scripts/ci/README.md +++ b/scripts/ci/README.md @@ -13,7 +13,7 @@ CI and the local infrastructure — both of which the venue-parity contract | `preflight-docker.sh` | Same idea for Colima/docker: prune runner-unlike residue, assert free space on the filesystem backing the docker data root (not the VM's `/`). Build cache + named volumes KEPT (the local analogue of actions/cache); `--deep` drops them. | `test-infrastructure/run.sh` | | `check-glibc-compat.sh` | Run a linux binary in debian:bullseye (glibc 2.31) — the portable binary must start on old glibc. | `_smoke.yml` portable legs | | `generate-sbom.py` | The release SPDX SBOM (vendored versions reviewable here, diffable by vendoring PRs — was inline YAML). | `release.yml` | -| `self-index-coverage-gate.sh` | Index this repo with the binary just built and fail the PR if its own parse-coverage report stops being useful advice: no whole-file failure, no range list clipped without saying so, no single range over 25% of a file of 200+ lines, and the flagged-file count at or below the checked-in ceiling. Reads its two data files, `coverage-gate-allowlist.txt` (paths it skips, each with a written reason) and `parse-partial-baseline.txt` (the ceiling). Ubuntu leg only — the flagged lines depend on which conditional-compilation branches the preprocessor keeps. | `pr.yml pr-smoke` | +| `self-index-coverage-gate.sh` | Index the merge base and this branch with the binary just built, and fail the PR only for a finding the branch ADDED (#963): a whole-file parse failure, a range list clipped without saying so, a single range over 25% of a file of 200+ lines, or a flagged-file count above the merge base's. Comparing against the base rather than a fixed number stops main moving the baseline and turning unrelated PRs red (#1972). Reads two data files: `coverage-gate-allowlist.txt` (paths it skips, each with a written reason) and `parse-partial-baseline.txt` (a record it prints, never a gate). Both trees are indexed with the SAME binary, so it cannot see an extractor change that moves both sides together. Ubuntu leg only — the flagged lines depend on which conditional-compilation branches the preprocessor keeps. | `pr.yml pr-smoke` | | `require-all-green.sh` | The aggregate gate: fail unless every needed job succeeded or legitimately skipped (was inline YAML). | `pr.yml ci-ok` | | `verify-shard-union.sh` | Prove sharded test legs lost nothing: shard count agreement, indices 1..n, identical suite lists, union of slices == full list (was inline YAML). | `_test.yml` shard-completeness | | `prepare-release-candidates.sh` | Copy one linker output into stripped/unstripped candidates, finalize signatures, composition-check them without execution, and record their hashes. | `_build.yml`, local artifact smoke | diff --git a/scripts/ci/parse-partial-baseline.txt b/scripts/ci/parse-partial-baseline.txt index b5d22da69..0f9d95688 100644 --- a/scripts/ci/parse-partial-baseline.txt +++ b/scripts/ci/parse-partial-baseline.txt @@ -1,9 +1,20 @@ -# Ceiling for parse_partial_count when this repo indexes itself. +# A written record of parse_partial_count when this repo indexes itself. # -# The number below is what the gate allows. It complements the FLOOR asserted -# in tests/test_index_resilience.c, which stops the signal being switched off -# by accident. Raising this number is allowed but should be explained in the -# commit that does it. +# NOTHING FAILS ON THIS NUMBER. The gate reports it and moves on. +# +# It used to be a ceiling. main could move the count on its own and every open +# pull request then went red for a reason none of them caused — that is #1972, +# where main gained src/daemon/runtime.c and the count went 58 -> 59 with no +# branch involved. The gate now indexes the merge base and compares against +# that, so the number below does not gate anything. +# +# What it is still for: a reader can see how far the count has drifted since +# anyone last looked, which is the one thing a merge-base comparison cannot +# show. Update it when you have measured a new number and can say why it moved. +# +# It complements the FLOOR asserted in tests/test_index_resilience.c, which +# stops the coverage signal being switched off by accident. That one IS +# enforced. # # 58 -> 59 on 2026-08-31. main gained src/daemon/runtime.c, whose line 47 the # tree-sitter C grammar cannot parse. The rise came from main, not from a diff --git a/scripts/ci/self-index-coverage-gate.sh b/scripts/ci/self-index-coverage-gate.sh index 00662a2b6..3e2a35432 100755 --- a/scripts/ci/self-index-coverage-gate.sh +++ b/scripts/ci/self-index-coverage-gate.sh @@ -7,7 +7,25 @@ # happened here before (#963): src/cli/cli.c reported its whole 13,046 lines # as one range, and two caps in series dropped ranges with no signal at all. # -# This indexes the repo with a given binary and fails if any of that comes back. +# THIS GATE COMPARES AGAINST THE MERGE BASE, NOT AGAINST A FIXED NUMBER. +# It indexes the base tree and the head tree with the SAME binary and fails +# only on a finding the branch added. An earlier version failed against a +# ceiling checked into a file, which meant main could move the number and turn +# every open pull request red for a reason none of them caused. That happened +# once already (#1972, the count went 58 -> 59 because main gained a file), +# and #1824 will do it again and larger — Blazor .razor files map to C#, the +# markup lands in ERROR regions by design, and the count rises by roughly the +# repo's .razor count. A coverage improvement must not read as a gate failure. +# +# WHAT THIS CANNOT SEE. Both trees are indexed with the same binary, so the +# comparison isolates what the TREE changed. A branch that changes the +# extractor itself moves the base side and the head side together, and this +# gate will not fail on it. Catching that needs the base commit's own binary, +# which means a second full build — about twelve minutes against the twenty-six +# seconds this whole step takes. What still covers it: the FLOOR asserted in +# tests/test_index_resilience.c stops the signal being switched off, and the +# absolute counts for both sides are printed below on every run, so a jump is +# visible in the log even when it does not fail. # # Usage: self-index-coverage-gate.sh # @@ -21,21 +39,28 @@ usage() { cat <<'EOF' Usage: scripts/ci/self-index-coverage-gate.sh -Index THIS repository with the given binary and fail if the repository's own -parse-coverage report stops being useful advice (#963). +Index the merge base and this branch with the given binary, and fail when the +branch made the repository's own parse-coverage report worse (#963). + +Four checks. Each one reads index_status for both trees and fails only on a +finding present on this branch and absent at the merge base: + 1. A file reports a whole-file parse failure (parse_unusable). + 2. A range list was clipped without saying so (a trailing "+" marker). + 3. A single range covers more than 25% of a file of 200 lines or more. + 4. The flagged-file count is higher than the merge base's count. -Four checks, all read from index_status: - 1. No file reports a whole-file parse failure (parse_unusable). - 2. No range list was clipped without saying so (a trailing "+" marker). - 3. No single range covers more than 25% of a file of 200 lines or more. - 4. The flagged-file count stays at or below the checked-in ceiling. +The base commit is taken from COVERAGE_GATE_BASE_SHA, or from the first parent +when HEAD is a merge commit (CI checks out refs/pull/N/merge), or from +`git merge-base origin/main HEAD`. Data files, both beside this script: coverage-gate-allowlist.txt paths the checks skip, each with a written reason above it. - parse-partial-baseline.txt the ceiling for check 4. + parse-partial-baseline.txt a written record of the count, reported but + NOT enforced. Nothing fails on it. Environment: + COVERAGE_GATE_BASE_SHA The commit to compare against. MAX_SINGLE_RANGE_PCT Share limit for check 3 (default 25). MIN_FILE_LINES Files below this are exempt from check 3 (default 200). @@ -82,102 +107,178 @@ WORK="$(mktemp -d)" # blows that limit and fails with "secure CLI coordination could not be # created (endpoint)". Keep the runtime dir short and separate from the cache. RUNTIME="/tmp/cbm-gate.$$" -trap 'rm -rf "$WORK" "$RUNTIME"' EXIT -export CBM_CACHE_DIR="${WORK}/cache" +BASE_TREE="${WORK}/base-tree" +cleanup() { + # The base tree is a real git worktree, so it has a registration in the + # repository that outlives a plain rm -rf. Retire it first, then the dirs. + if [ -d "$BASE_TREE" ]; then + git -C "$REPO_ROOT" worktree remove --force "$BASE_TREE" >/dev/null 2>&1 || true + fi + git -C "$REPO_ROOT" worktree prune >/dev/null 2>&1 || true + rm -rf "$WORK" "$RUNTIME" +} +trap cleanup EXIT export CBM_RUNTIME_DIR="$RUNTIME" -mkdir -p "$CBM_CACHE_DIR" "$RUNTIME" +mkdir -p "$RUNTIME" + +FAILURES=0 +note_failure() { echo "FAIL: $*"; FAILURES=$((FAILURES + 1)); } + +# ── Which commit are we comparing against? ──────────────────────────────── +# CI checks out refs/pull/N/merge, so the first parent IS the base commit. +# The env var comes from the workflow and wins, because it is the value +# GitHub itself used to build that merge. +resolve_base() { + if [ -n "${COVERAGE_GATE_BASE_SHA:-}" ]; then + printf '%s' "$COVERAGE_GATE_BASE_SHA" + return 0 + fi + if git -C "$REPO_ROOT" rev-parse --verify -q 'HEAD^2' >/dev/null 2>&1; then + git -C "$REPO_ROOT" rev-parse 'HEAD^1' + return 0 + fi + git -C "$REPO_ROOT" merge-base origin/main HEAD 2>/dev/null || return 1 +} -echo "==> indexing ${REPO_ROOT} with $(basename "$BIN")" -"$BIN" cli index_repository --repo-path "$REPO_ROOT" --mode full --json \ - > "${WORK}/index.json" 2>"${WORK}/index.err" || { - echo "FAIL: index_repository exited non-zero"; tail -20 "${WORK}/index.err"; exit 1; } +BASE_SHA="$(resolve_base || true)" +[ -n "$BASE_SHA" ] || { echo "FAIL: could not work out the base commit — set COVERAGE_GATE_BASE_SHA"; exit 1; } -PROJECT="$(jq -r '.structuredContent.project // empty' "${WORK}/index.json")" -[ -n "$PROJECT" ] || { echo "FAIL: index_repository did not name a project"; exit 1; } +# The CI checkout is shallow, so the base commit's tree may not be present. +if ! git -C "$REPO_ROOT" cat-file -e "${BASE_SHA}^{tree}" 2>/dev/null; then + echo "==> fetching base commit ${BASE_SHA}" + git -C "$REPO_ROOT" fetch --no-tags --depth=1 origin "$BASE_SHA" >/dev/null 2>&1 || { + echo "FAIL: could not fetch base commit ${BASE_SHA}"; exit 1; } +fi -"$BIN" cli index_status --project "$PROJECT" --json > "${WORK}/status.json" 2>/dev/null || { - echo "FAIL: index_status exited non-zero"; exit 1; } +# A real worktree rather than an archive: the head side IS a git checkout, and +# the indexer's git passes must see the same shape on both sides. It lives +# outside REPO_ROOT so the head index never walks into it. +git -C "$REPO_ROOT" worktree add --detach "$BASE_TREE" "$BASE_SHA" >/dev/null 2>&1 || { + echo "FAIL: could not check out base commit ${BASE_SHA}"; exit 1; } -# Allowlisted paths, comments and blanks stripped. +# Allowlisted paths, comments and blanks stripped. Applied to BOTH sides, so +# an allowlisted path can neither fail the branch nor mask a base finding. ALLOWED="${WORK}/allowed.txt" : > "$ALLOWED" [ -f "$ALLOWLIST" ] && sed -e 's/#.*//' -e 's/[[:space:]]*$//' "$ALLOWLIST" \ | grep -v '^$' > "$ALLOWED" || true -FAILURES=0 -note_failure() { echo "FAIL: $*"; FAILURES=$((FAILURES + 1)); } +# ── Index one tree and write its findings as sorted path lists ──────────── +# Writes .unusable, .truncated, .wide and +# .count. Fails the run outright when index_status clipped its own +# file list, because the three list-based checks would then judge only part of +# the tree and still print PASS — the same silent clipping this gate exists to +# catch. +analyze_tree() { + tree_root="$1" + prefix="$2" + label="$3" -# ── 0. The report's own file list must be complete ──────────────────────── -# index_status lists at most 500 files per class and sets "truncated" when it -# drops the rest. Checks 2 and 3 below read that list file by file, so a -# clipped list means they judge only part of the repo and still print PASS. -# That is the same silent clipping this gate exists to catch, so stop instead. -for cls in parse_partial parse_unusable; do - clipped="$(jq -r --arg c "$cls" '.structuredContent[$c].truncated // false' "${WORK}/status.json")" - if [ "$clipped" = "true" ]; then - note_failure "index_status clipped its ${cls} file list — the checks below would see only part of it" - fi -done + cache="${WORK}/cache-${label}" + mkdir -p "$cache" + export CBM_CACHE_DIR="$cache" -# ── 1. Nothing may fail across a whole file ──────────────────────────────── -# parse_unusable means one range covers 80%+ of the file, so the report tells -# a reader to go read the source. Zero today; a new one is a real regression. -UNUSABLE_LISTED="$(jq -r '[.structuredContent.parse_unusable.files[]?.path]|join(" ")' \ - "${WORK}/status.json")" -UNUSABLE=0 -UNUSABLE_KEPT="" -for p in $UNUSABLE_LISTED; do - grep -qxF "$p" "$ALLOWED" && continue - UNUSABLE=$((UNUSABLE + 1)) - UNUSABLE_KEPT="${UNUSABLE_KEPT}${p} " -done -if [ "$UNUSABLE" -gt 0 ]; then - note_failure "${UNUSABLE} file(s) report a whole-file parse failure: ${UNUSABLE_KEPT}" -fi + echo "==> indexing ${label} tree with $(basename "$BIN")" + "$BIN" cli index_repository --repo-path "$tree_root" --mode full --json \ + > "${prefix}.index.json" 2>"${prefix}.index.err" || { + echo "FAIL: index_repository exited non-zero for the ${label} tree" + tail -20 "${prefix}.index.err"; exit 1; } -# ── 2. No range list may be silently clipped ────────────────────────────── -# A trailing "+" says the producer's cap threw N ranges away. With the cap -# at 256 a file that still overflows is worth stopping for. -TRUNCATED="$(jq -r '[.structuredContent.parse_partial.files[]? - | select(.error_ranges? // "" | test("\\+[0-9]+$")) | .path] | join(" ")' \ - "${WORK}/status.json")" -for p in $TRUNCATED; do - grep -qxF "$p" "$ALLOWED" && continue - note_failure "$p carries a +N truncation marker — its range list was clipped" -done + project="$(jq -r '.structuredContent.project // empty' "${prefix}.index.json")" + [ -n "$project" ] || { echo "FAIL: index_repository did not name a ${label} project"; exit 1; } + + "$BIN" cli index_status --project "$project" --json > "${prefix}.status.json" 2>/dev/null || { + echo "FAIL: index_status exited non-zero for the ${label} tree"; exit 1; } + + status="${prefix}.status.json" + + for cls in parse_partial parse_unusable; do + clipped="$(jq -r --arg c "$cls" '.structuredContent[$c].truncated // false' "$status")" + if [ "$clipped" = "true" ]; then + note_failure "index_status clipped its ${cls} file list on the ${label} tree — the checks below would see only part of it" + fi + done + + # 1. Whole-file parse failures. + jq -r '.structuredContent.parse_unusable.files[]?.path' "$status" \ + | grep -vxF -f "$ALLOWED" 2>/dev/null | sort -u > "${prefix}.unusable" || : > "${prefix}.unusable" + + # 2. Range lists the producer's cap clipped, marked with a trailing "+". + jq -r '.structuredContent.parse_partial.files[]? + | select(.error_ranges? // "" | test("\\+[0-9]+$")) | .path' "$status" \ + | grep -vxF -f "$ALLOWED" 2>/dev/null | sort -u > "${prefix}.truncated" || : > "${prefix}.truncated" + + # 3. One range covering more than its share of the file. The line count + # comes from the tree being analysed, because a file can grow or shrink + # between the two commits. + : > "${prefix}.wide" + while IFS=$'\t' read -r path ranges; do + [ -n "$path" ] || continue + grep -qxF "$path" "$ALLOWED" && continue + [ -f "${tree_root}/${path}" ] || continue + total="$(wc -l < "${tree_root}/${path}" | tr -d ' ')" + [ "$total" -ge "$MIN_FILE_LINES" ] || continue + widest="$(printf '%s' "$ranges" | tr ',' '\n' | grep '^[0-9]' \ + | awk -F- '{d=$2-$1+1; if (d>m) m=d} END {print m+0}')" + pct="$(awk -v a="$widest" -v b="$total" 'BEGIN{printf "%.1f", 100*a/b}')" + over="$(awk -v p="$pct" -v lim="$MAX_SINGLE_RANGE_PCT" 'BEGIN{print (p>lim)?1:0}')" + if [ "$over" = "1" ]; then + printf '%s\t%s\t%s\t%s\n' "$path" "$widest" "$pct" "$total" >> "${prefix}.wide" + fi + done < <(jq -r '.structuredContent.parse_partial.files[]? + | "\(.path)\t\(.error_ranges // "")"' "$status") + sort -u -o "${prefix}.wide" "${prefix}.wide" + + # 4. The flagged-file count. + jq -r '.structuredContent.parse_partial.count // 0' "$status" > "${prefix}.count" +} -# ── 3. No single range may cover a quarter of its file ──────────────────── -while IFS=$'\t' read -r path ranges; do +analyze_tree "$BASE_TREE" "${WORK}/base" base +analyze_tree "$REPO_ROOT" "${WORK}/head" head + +BASE_COUNT="$(cat "${WORK}/base.count")" +HEAD_COUNT="$(cat "${WORK}/head.count")" + +# ── The four checks, each on what the branch ADDED ──────────────────────── +while read -r p; do + [ -n "$p" ] || continue + note_failure "$p reports a whole-file parse failure on this branch and not at the merge base" +done < <(comm -13 "${WORK}/base.unusable" "${WORK}/head.unusable") + +while read -r p; do + [ -n "$p" ] || continue + note_failure "$p carries a +N truncation marker on this branch and not at the merge base — its range list was clipped" +done < <(comm -13 "${WORK}/base.truncated" "${WORK}/head.truncated") + +# Compare by path, not by the whole row: a file already over the share at the +# merge base must not fail here just because the range moved by a line. +cut -f1 "${WORK}/base.wide" | sort -u > "${WORK}/base.wide.paths" +while IFS=$'\t' read -r path widest pct total; do [ -n "$path" ] || continue - grep -qxF "$path" "$ALLOWED" && continue - [ -f "${REPO_ROOT}/${path}" ] || continue - total="$(wc -l < "${REPO_ROOT}/${path}" | tr -d ' ')" - [ "$total" -ge "$MIN_FILE_LINES" ] || continue - widest="$(printf '%s' "$ranges" | tr ',' '\n' | grep '^[0-9]' \ - | awk -F- '{d=$2-$1+1; if (d>m) m=d} END {print m+0}')" - pct="$(awk -v a="$widest" -v b="$total" 'BEGIN{printf "%.1f", 100*a/b}')" - over="$(awk -v p="$pct" -v lim="$MAX_SINGLE_RANGE_PCT" 'BEGIN{print (p>lim)?1:0}')" - if [ "$over" = "1" ]; then - note_failure "$path has one range of ${widest} lines — ${pct}% of ${total}, over ${MAX_SINGLE_RANGE_PCT}%" - fi -done < <(jq -r '.structuredContent.parse_partial.files[]? - | "\(.path)\t\(.error_ranges // "")"' "${WORK}/status.json") - -# ── 4. The flagged-file count must not drift upward unnoticed ───────────── -# One awk, not `grep | head -1`: under `set -o pipefail` head closes the pipe -# early, grep dies of SIGPIPE, and the whole gate aborts with no message. It -# does not bite on today's short file, and it would bite the day someone adds -# a few comment lines. -CEILING="$(awk '{ sub(/#.*/, "") } match($0, /[0-9]+/) { print substr($0, RSTART, RLENGTH); exit }' \ - "$BASELINE_FILE")" -PARTIAL="$(jq -r '.structuredContent.parse_partial.count // 0' "${WORK}/status.json")" -if [ "$PARTIAL" -gt "$CEILING" ]; then - note_failure "parse_partial_count is ${PARTIAL}, above the ceiling ${CEILING} in $(basename "$BASELINE_FILE")" + grep -qxF "$path" "${WORK}/base.wide.paths" && continue + note_failure "$path has one range of ${widest} lines — ${pct}% of ${total}, over ${MAX_SINGLE_RANGE_PCT}%, and it was within the share at the merge base" +done < "${WORK}/head.wide" + +if [ "$HEAD_COUNT" -gt "$BASE_COUNT" ]; then + note_failure "parse_partial_count is ${HEAD_COUNT} on this branch against ${BASE_COUNT} at the merge base" +fi + +# ── Report ─────────────────────────────────────────────────────────────── +# parse-partial-baseline.txt is a written record, not a gate. It is printed so +# a reader can see the count drift, and nothing fails on it: enforcing it is +# what turned an unrelated branch red when main moved (#1972). +RECORDED="$(awk '{ sub(/#.*/, "") } match($0, /[0-9]+/) { print substr($0, RSTART, RLENGTH); exit }' \ + "$BASELINE_FILE" 2>/dev/null || echo "")" +echo "==> base ${BASE_SHA}: parse_partial=${BASE_COUNT} unusable=$(wc -l < "${WORK}/base.unusable" | tr -d ' ')" +echo "==> head: parse_partial=${HEAD_COUNT} unusable=$(wc -l < "${WORK}/head.unusable" | tr -d ' ')" +echo "==> allowlisted=$(wc -l < "$ALLOWED" | tr -d ' ') recorded=${RECORDED:-none} (record only, not enforced)" +if [ -n "$RECORDED" ] && [ "$HEAD_COUNT" != "$RECORDED" ]; then + echo "NOTE: parse_partial_count is ${HEAD_COUNT}, and $(basename "$BASELINE_FILE") records ${RECORDED}." + echo "NOTE: that is not a failure. If this branch changed the extractor, both sides above moved together and this gate cannot tell." fi -echo "==> parse_partial=${PARTIAL} (ceiling ${CEILING}) parse_unusable=${UNUSABLE} allowlisted=$(wc -l < "$ALLOWED" | tr -d ' ')" if [ "$FAILURES" -gt 0 ]; then echo "FAIL: ${FAILURES} coverage-gate check(s) failed" exit 1 fi -echo "PASS: parse-coverage report is within bounds" +echo "PASS: this branch did not make the parse-coverage report worse than the merge base" diff --git a/scripts/test.sh b/scripts/test.sh index 0aafe6c20..0f4d31dfc 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -272,6 +272,9 @@ bash "$ROOT/tests/test_language_count_contract.sh" echo "=== Step 0x: packaging version-metadata contract ===" bash "$ROOT/tests/test_version_metadata_contract.sh" +echo "=== Step 0y: parse-coverage gate contract (#963) ===" +bash "$ROOT/tests/test_coverage_gate_contract.sh" + # Verify compiler supports target arch verify_compiler "$CC" diff --git a/tests/test_coverage_gate_contract.sh b/tests/test_coverage_gate_contract.sh new file mode 100755 index 000000000..8755fbfe5 --- /dev/null +++ b/tests/test_coverage_gate_contract.sh @@ -0,0 +1,241 @@ +#!/usr/bin/env bash +# Contract: scripts/ci/self-index-coverage-gate.sh fails a branch only for a +# finding the branch ADDED (#963, #1972). +# +# The gate used to compare against a number checked into the repository. Main +# could move that number on its own, and every open pull request went red for +# a reason none of them caused. The gate now indexes the merge base and the +# branch with the same binary and compares the two. This test pins that. +# +# No seam is added to the production script. It calls the binary as +# "$BIN cli index_repository …" and "$BIN cli index_status …", so a fake $BIN +# that prints canned JSON exercises every check without indexing anything. +# Same idea as tests/repro/repro_script_summary.sh. +set -euo pipefail +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +WORKDIR="$(mktemp -d)" +cleanup() { + # Each gate run makes its own worktree and removes it. A run that dies + # early can leave a registration behind, so prune before deleting. + git -C "$REPO" worktree prune >/dev/null 2>&1 || true + rm -rf "$WORKDIR" +} +REPO="$WORKDIR/repo" +trap cleanup EXIT +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +FIXTURE="$WORKDIR/fixture" +mkdir -p "$FIXTURE" + +# ── A repository with a base commit and a head commit ───────────────────── +mkdir -p "$REPO/scripts/ci" "$REPO/src" +cp "$ROOT/scripts/ci/self-index-coverage-gate.sh" "$REPO/scripts/ci/" +printf '# no allowlisted paths\n' > "$REPO/scripts/ci/coverage-gate-allowlist.txt" +printf '# recorded count\n7\n' > "$REPO/scripts/ci/parse-partial-baseline.txt" +# Check 3 measures a range against the file's own length, and exempts +# anything under 200 lines. These two are long enough to be measured. +for f in big allowed; do + awk 'BEGIN { for (i = 1; i <= 300; i++) print "int line_" i ";" }' > "$REPO/src/$f.c" +done +git -C "$REPO" init -q +git -C "$REPO" config user.email "gate@test.invalid" +git -C "$REPO" config user.name "Gate Test" +git -C "$REPO" config commit.gpgsign false +# The fake binary reads this file to tell which tree it was pointed at. +printf 'base\n' > "$REPO/.side" +git -C "$REPO" add -A +git -C "$REPO" -c core.hooksPath=/dev/null commit -qm "base" +BASE_SHA="$(git -C "$REPO" rev-parse HEAD)" +printf 'head\n' > "$REPO/.side" +git -C "$REPO" add -A +git -C "$REPO" -c core.hooksPath=/dev/null commit -qm "head" + +# ── The fake binary ─────────────────────────────────────────────────────── +# index_repository answers with the tree's own side as the project name, so +# the index_status call that follows can be answered for the right side. +FAKE_BIN="$WORKDIR/fake-cbm" +cat > "$FAKE_BIN" <<'FAKE_EOF' +#!/usr/bin/env bash +set -euo pipefail +sub="" +repo_path="" +project="" +while [ $# -gt 0 ]; do + case "$1" in + index_repository | index_status) sub="$1" ;; + --repo-path) repo_path="$2"; shift ;; + --project) project="$2"; shift ;; + esac + shift +done +case "$sub" in +index_repository) + side="$(tr -d '[:space:]' < "${repo_path}/.side")" + printf '{"structuredContent":{"project":"%s"}}\n' "$side" + ;; +index_status) + cat "${GATE_FIXTURE_DIR}/${project}.json" + ;; +*) + echo "fake-cbm: unexpected call" >&2 + exit 1 + ;; +esac +FAKE_EOF +chmod +x "$FAKE_BIN" + +# ── One gate run ────────────────────────────────────────────────────────── +# $1 case name, $2 expected outcome (pass|fail), $3 base JSON, $4 head JSON. +run_case() { + name="$1" + expect="$2" + printf '%s\n' "$3" > "${FIXTURE}/base.json" + printf '%s\n' "$4" > "${FIXTURE}/head.json" + out="$( + COVERAGE_GATE_BASE_SHA="$BASE_SHA" \ + GATE_FIXTURE_DIR="$FIXTURE" \ + bash "$REPO/scripts/ci/self-index-coverage-gate.sh" "$FAKE_BIN" 2>&1 + )" && rc=0 || rc=$? + if [ "$expect" = "pass" ] && [ "$rc" -ne 0 ]; then + printf '%s\n' "$out" >&2 + fail "${name}: expected the gate to pass, it exited ${rc}" + fi + if [ "$expect" = "fail" ] && [ "$rc" -eq 0 ]; then + printf '%s\n' "$out" >&2 + fail "${name}: expected the gate to fail, it exited 0" + fi + LAST_OUT="$out" + echo "ok: ${name}" +} + +# ── JSON builders ───────────────────────────────────────────────────────── +# $1 partial count, $2 partial files array, $3 unusable files array, +# $4 partial truncated flag. +status() { + cat < base ${BASE_SHA}: parse_partial=40"*) ;; +*) fail "the base side was not printed" ;; +esac +case "$LAST_OUT" in +*"==> head: parse_partial=40"*) ;; +*) fail "the head side was not printed" ;; +esac + +# ── A clipped file list still stops the run outright ────────────────────── +# This check cannot be differential. A clipped list means the three list-based +# checks above saw only part of the tree and would print PASS anyway. +run_case "a clipped file list at head fails" fail \ + "$(status 3 "$NARROW" "$NO_FILES")" "$(status 3 "$NARROW" "$NO_FILES" true)" +case "$LAST_OUT" in +*"clipped its parse_partial file list on the head tree"*) ;; +*) fail "the clipped-list failure did not name the tree" ;; +esac + +# ── The allowlist skips a path on both sides ────────────────────────────── +printf '# a written reason belongs above each path\nsrc/allowed.c\n' \ + > "$REPO/scripts/ci/coverage-gate-allowlist.txt" +git -C "$REPO" add -A +git -C "$REPO" -c core.hooksPath=/dev/null commit -qm "allowlist src/allowed.c" +run_case "an allowlisted path newly over the share passes" pass \ + "$(status 3 "$NARROW" "$NO_FILES")" "$(status 3 "$WIDE_ALLOWED" "$NO_FILES")" + +# ── The base commit has to be resolvable ────────────────────────────────── +printf '%s\n' "$(status 3 "$NARROW" "$NO_FILES")" > "${FIXTURE}/base.json" +printf '%s\n' "$(status 3 "$NARROW" "$NO_FILES")" > "${FIXTURE}/head.json" +out="$( + COVERAGE_GATE_BASE_SHA="0000000000000000000000000000000000000000" \ + GATE_FIXTURE_DIR="$FIXTURE" \ + bash "$REPO/scripts/ci/self-index-coverage-gate.sh" "$FAKE_BIN" 2>&1 +)" && rc=0 || rc=$? +[ "$rc" -ne 0 ] || fail "an unreachable base commit must not pass" +case "$out" in +*"base commit"*) ;; +*) fail "an unreachable base commit did not say so: $out" ;; +esac +echo "ok: an unreachable base commit stops the run" + +# ── The interface contract ──────────────────────────────────────────────── +out="$(bash "$REPO/scripts/ci/self-index-coverage-gate.sh" --help 2>&1)" || fail "--help exited non-zero" +case "$out" in +*"Usage:"*) ;; +*) fail "--help printed no Usage: block" ;; +esac +case "$out" in +*"merge base"*) ;; +*) fail "--help does not describe the merge-base comparison" ;; +esac +echo "ok: --help describes the merge-base comparison" + +echo "PASS: coverage-gate contract"