From f513165bacd044a933243ec65257a6629e66e40e Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sat, 29 Aug 2026 23:48:15 -0400 Subject: [PATCH 1/4] 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 423406612..b5e92d7dc 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 */ + /* ── Tests ────────────────────────────────────────────────────────────────── */ TEST(c_ifdef_split_brace_sets_parse_incomplete) { @@ -418,6 +441,117 @@ TEST(width_bearing_error_at_eof_still_flagged_issue1610) { 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); @@ -437,4 +571,9 @@ 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); } From 5ee9266915c1b99ba0ff737a7e78efd136db0c9b Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 00:32:39 -0400 Subject: [PATCH 2/4] 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 67df96831..9a6a4583c 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -386,9 +386,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 " @@ -489,7 +491,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 " @@ -642,6 +645,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 " @@ -4546,10 +4551,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; @@ -4564,6 +4571,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); @@ -4596,6 +4615,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); @@ -4638,7 +4665,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 */ @@ -4750,6 +4778,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; @@ -4757,10 +4793,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; } @@ -4792,9 +4833,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, @@ -4808,7 +4855,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); @@ -4834,6 +4882,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"; } @@ -4861,6 +4915,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"; } @@ -7787,6 +7846,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: @@ -7794,13 +7866,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++; } } @@ -7815,7 +7887,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); @@ -7874,6 +7946,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 @@ -7917,6 +8036,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; @@ -7994,6 +8114,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; @@ -9128,10 +9249,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) { @@ -9144,18 +9265,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 1eeb55f83..85274392c 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 ee58d10f6..2272497a5 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 b5e92d7dc..baac77e0c 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) { @@ -221,12 +222,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; @@ -245,6 +256,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. */ @@ -552,6 +612,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); @@ -561,6 +677,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 8a973281608cc5d718da4d811a03bc0d4e5b7b5e Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Sun, 30 Aug 2026 01:35:52 -0400 Subject: [PATCH 3/4] 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 9a6a4583c..9cbf3746a 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -492,7 +492,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 " @@ -645,8 +646,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 " @@ -4855,8 +4856,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 e686ac3b7..9f9722fcc 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -856,6 +856,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 cacd4d88d..6518347d2 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -2750,6 +2750,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)); @@ -13571,6 +13651,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 baac77e0c..8fd7724a2 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 @@ -668,6 +668,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); @@ -698,4 +741,5 @@ SUITE(parse_coverage) { 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(c_thread_local_grammar_limit_is_pinned_issue963); } diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index a45541e57..f74e21ed1 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -12751,6 +12751,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 */ @@ -12795,6 +12876,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 5ef813136705707c6896867e08c82a10400ce105 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Mon, 31 Aug 2026 14:54:39 -0400 Subject: [PATCH 4/4] 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 9cbf3746a..63d7faab9 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -4579,8 +4579,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++; @@ -7950,13 +7953,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; @@ -7979,9 +7982,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); }