Skip to content

fix(coverage): narrow parse-error ranges, and stop the report hiding what it dropped (#963) #1941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CaptainMittens wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(coverage): narrow parse-error ranges, and stop the report hiding what it dropped (#963) #1941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cb167b9
fix(coverage): narrow parse-error ranges with the preprocessed parse …
CaptainMittens Aug 30, 2026
a953509
fix(coverage): stop the report hiding what it dropped, and name whole…
CaptainMittens Aug 30, 2026
219b348
test(coverage): pin the truncation marker, the partial ceiling and th…
CaptainMittens Aug 30, 2026
f871178
fix(coverage): name the whole-file number for what it is, a range end…
CaptainMittens Aug 31, 2026
b04f545
perf(coverage): find a line by table lookup, not by rescanning the fi…
CaptainMittens Sep 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Makefile.cbm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
444 changes: 413 additions & 31 deletions internal/cbm/cbm.c
View file
Open in desktop

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions internal/cbm/cbm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,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;
Expand Down
167 changes: 149 additions & 18 deletions src/mcp/mcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6032,10 +6032,12 @@ static void add_coverage_report(yyjson_mut_doc *doc, yyjson_mut_val *root, cbm_s
bool have_meta = cbm_store_coverage_meta_get(store, project, &meta) == CBM_STORE_OK;

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;
Expand All @@ -6050,6 +6052,21 @@ 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);
/* 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, "range_end", 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 < sample_limit) {
yyjson_mut_arr_add_strcpy(doc, ni_dirs, rows[i].rel_path);
Expand Down Expand Up @@ -6099,6 +6116,14 @@ static void add_coverage_report(yyjson_mut_doc *doc, yyjson_mut_val *root, cbm_s
}
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);
Expand Down Expand Up @@ -6153,7 +6178,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 */
Expand Down Expand Up @@ -6265,17 +6291,30 @@ 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,...[,+<N>]" string into a JSON ranges array.
*
* The optional trailing "+<N>" 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
* "ranges_truncated": true whenever ranges were lost — either by that marker,
* or by COVERAGE_RANGE_MAX stopping this loop. */
static bool coverage_add_ranges(yyjson_mut_doc *doc, yyjson_mut_val *row, const char *detail) {
if (!detail || !detail[0]) {
return false;
}
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;
}
Expand Down Expand Up @@ -6307,11 +6346,17 @@ static bool 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 (emitted == COVERAGE_RANGE_MAX && *p) {
yyjson_mut_obj_add_bool(doc, row, "ranges_truncated", true);
}
}
/* Losing ranges to the producer's cap and losing them to this reader's cap
* are the same fact for a caller: the list is short and what is missing is
* unknown. One flag says so, whether or not any range survived. */
if (truncated) {
yyjson_mut_obj_add_bool(doc, row, "ranges_truncated", true);
}
return emitted > 0;
}
Expand All @@ -6328,7 +6373,8 @@ static void coverage_add_row_json(yyjson_mut_doc *doc, yyjson_mut_val *array,
row->rel_path && strcmp(row->rel_path, requested_path) == 0 ? "exact" : "ancestor");
}
bool ranges_added = false;
if (row->kind && strcmp(row->kind, "parse_partial") == 0) {
if (row->kind &&
(strcmp(row->kind, "parse_partial") == 0 || strcmp(row->kind, "parse_unusable") == 0)) {
ranges_added = coverage_add_ranges(doc, item, row->detail);
}
/* A parse_partial detail such as "3-4,9" is byte-for-byte redundant once
Expand Down Expand Up @@ -6361,6 +6407,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";
}
Expand Down Expand Up @@ -6388,6 +6440,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";
}
Expand Down Expand Up @@ -9967,20 +10024,33 @@ 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:
* "skipped": {"files":[{path,reason,phase}..(<=50)], "count":N, "truncated":bool}
* and, if a per-run logfile was written, "logfile": "<path>".
* 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++;
}
}
Expand All @@ -9995,7 +10065,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);
Expand Down Expand Up @@ -10054,6 +10124,55 @@ 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,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 — "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;
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 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, "range_end", 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
Expand Down Expand Up @@ -10097,6 +10216,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;
Expand Down Expand Up @@ -10174,6 +10294,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;
Expand Down Expand Up @@ -11335,10 +11456,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) {
Expand All @@ -11351,18 +11472,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);
}
Expand Down
Loading
Loading