Skip to content

Commit db2e76f

Browse files
fix(coverage): stop the report hiding what it dropped, and name whole-file failures (#963)
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 ",+<N>" 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VgDHuhXmjdrwzowPN68wsC
1 parent c224c9b commit db2e76f

7 files changed

Lines changed: 447 additions & 45 deletions

File tree

internal/cbm/cbm.c

Lines changed: 72 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -774,16 +774,24 @@ static bool cbm_source_nesting_exceeds(const char *source, int source_len, int c
774774
* nodes (does not descend into an error subtree — one range per failed region).
775775
* Bounded by CBM_MAX_ERROR_REGIONS so pathological input can't blow up the
776776
* output. The ranges mark where constructs were dropped; they are a detection
777-
* aid, never a completeness proof. */
778-
#define CBM_MAX_ERROR_REGIONS 64
777+
* aid, never a completeness proof.
778+
*
779+
* `dropped` counts the ranges the cap threw away. It exists so a clipped list
780+
* cannot read as a complete one: cbm_error_ranges_str turns a non-zero count
781+
* into a trailing "+<N>" marker. Phase 2 split one whole-file range into many
782+
* small ones, which pushed real files straight into a cap that used to be
783+
* unreachable, so the clip is live behaviour and not a theoretical limit. */
784+
#define CBM_MAX_ERROR_REGIONS 256
779785
typedef struct {
780786
uint32_t starts[CBM_MAX_ERROR_REGIONS];
781787
uint32_t ends[CBM_MAX_ERROR_REGIONS];
782788
int count;
789+
int dropped;
783790
} cbm_error_regions_t;
784791

785792
static void cbm_error_regions_push(cbm_error_regions_t *acc, TSNode n) {
786793
if (acc->count >= CBM_MAX_ERROR_REGIONS) {
794+
acc->dropped++;
787795
return;
788796
}
789797
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_
843851
return true;
844852
}
845853

854+
/* Walks to the end even after the cap is full, so `dropped` is the real number
855+
* of ranges lost rather than a lower bound. This costs little: the walk never
856+
* descends into an ERROR subtree — it records the top-most node and moves on —
857+
* so it only visits the spine of nodes that contain an error, plus one level. */
846858
static void cbm_collect_error_regions(TSNode n, cbm_error_regions_t *acc, const char *source,
847859
int source_len) {
848-
if (acc->count >= CBM_MAX_ERROR_REGIONS) {
849-
return;
850-
}
851860
uint32_t k = ts_node_child_count(n);
852-
for (uint32_t i = 0; i < k && acc->count < CBM_MAX_ERROR_REGIONS; i++) {
861+
for (uint32_t i = 0; i < k; i++) {
853862
TSNode c = ts_node_child(n, i);
854863
if (ts_node_is_missing(c) || strcmp(ts_node_type(c), "ERROR") == 0) {
855864
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
12901299
while (end >= start && end <= line_count && (map[end] & CBM_LINE_NO_CODE)) {
12911300
end--;
12921301
}
1293-
if (start > end || out->count >= CBM_MAX_ERROR_REGIONS) {
1302+
if (start > end) {
1303+
return; /* nothing but blank, comment or directive lines — no construct lost */
1304+
}
1305+
if (out->count >= CBM_MAX_ERROR_REGIONS) {
1306+
out->dropped++;
12941307
return;
12951308
}
12961309
out->starts[out->count] = start;
@@ -1322,7 +1335,7 @@ static bool cbm_line_is_toplevel_macro_call(const char *src, int src_len, uint32
13221335
static void cbm_refine_regions_with_pp_lines(cbm_error_regions_t *regs, const uint8_t *map,
13231336
uint32_t line_count, const char *src, int src_len,
13241337
const CBMDefArray *defs) {
1325-
cbm_error_regions_t out = {{0}, {0}, 0};
1338+
cbm_error_regions_t out = {{0}, {0}, 0, regs->dropped};
13261339
for (int i = 0; i < regs->count; i++) {
13271340
uint32_t run_start = 0;
13281341
uint32_t run_end = 0;
@@ -1349,12 +1362,40 @@ static void cbm_refine_regions_with_pp_lines(cbm_error_regions_t *regs, const ui
13491362
}
13501363

13511364
/* Serialize collected regions as "start-end,start-end,..." into the arena. */
1365+
/* Share of a file one range must cover before the range stops being advice and
1366+
* becomes noise. 80% is well clear of anything real: the widest single range in
1367+
* this repo covers 25.5% of its file, and the next widest 3.9%. */
1368+
#define CBM_UNUSABLE_PCT 80
1369+
1370+
/* Number of 1-based lines in `src`. A file that does not end with a newline
1371+
* still has a last line, so the count is separators plus one. */
1372+
static uint32_t cbm_count_lines(const char *src, int src_len) {
1373+
uint32_t n = 1;
1374+
for (int i = 0; i < src_len; i++) {
1375+
if (src[i] == '\n' && i + 1 < src_len) {
1376+
n++;
1377+
}
1378+
}
1379+
return n;
1380+
}
1381+
1382+
/* Serialize collected regions as "start-end,start-end,...", with a trailing
1383+
* ",+<N>" when the cap threw N ranges away.
1384+
*
1385+
* The marker must stay a SUFFIX and nothing else. Every reader stops at the
1386+
* first token that is not a range, so a marker in the middle of a string
1387+
* silently hides everything after it. objectscript_export_append_error_ranges
1388+
* strips markers before joining two parts for exactly that reason.
1389+
*
1390+
* N can be non-zero while the kept list is short, because the recovery and
1391+
* macro rules run after collection and remove ranges the cap never saw. That
1392+
* still reports honestly: the cap bound, so what was lost is unknown. */
13521393
static const char *cbm_error_ranges_str(CBMArena *a, const cbm_error_regions_t *regs) {
1353-
if (regs->count <= 0) {
1394+
if (regs->count <= 0 && regs->dropped <= 0) {
13541395
return NULL;
13551396
}
13561397
enum { RANGE_MAX = 24 }; /* "4294967295-4294967295," */
1357-
char *buf = (char *)cbm_arena_alloc(a, (size_t)regs->count * RANGE_MAX);
1398+
char *buf = (char *)cbm_arena_alloc(a, (size_t)(regs->count + 1) * RANGE_MAX);
13581399
if (!buf) {
13591400
return NULL;
13601401
}
@@ -1363,6 +1404,9 @@ static const char *cbm_error_ranges_str(CBMArena *a, const cbm_error_regions_t *
13631404
off += (size_t)snprintf(buf + off, RANGE_MAX, "%s%u-%u", i ? "," : "", regs->starts[i],
13641405
regs->ends[i]);
13651406
}
1407+
if (regs->dropped > 0) {
1408+
snprintf(buf + off, RANGE_MAX, "%s+%d", off ? "," : "", regs->dropped);
1409+
}
13661410
return buf;
13671411
}
13681412

@@ -1680,7 +1724,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
16801724
* the raw source line, and whose QN the raw pass did not
16811725
* already extract. */
16821726
if (ts_node_has_error(root)) {
1683-
cbm_error_regions_t raw_regs = {{0}, {0}, 0};
1727+
cbm_error_regions_t raw_regs = {{0}, {0}, 0, 0};
16841728
cbm_collect_error_regions(root, &raw_regs, source, source_len);
16851729
if (raw_regs.count > 0) {
16861730
int defs_before = result->defs.count;
@@ -1889,7 +1933,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
18891933
* miss, and a fully recovered file is not flagged at all. Detection aid
18901934
* only: the absence of this flag is NOT a completeness guarantee. */
18911935
if (ts_node_has_error(root)) {
1892-
cbm_error_regions_t regs = {{0}, {0}, 0};
1936+
cbm_error_regions_t regs = {{0}, {0}, 0, 0};
18931937
if (strcmp(ts_node_type(root), "ERROR") == 0) {
18941938
cbm_error_regions_push(&regs, root); /* whole file unparseable */
18951939
} else {
@@ -1914,10 +1958,25 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
19141958
* refinement, because its evidence is per-line: a narrow range points at
19151959
* the call itself instead of the whole blob around it. */
19161960
cbm_subtract_macro_invocation_regions(&regs, &result->defs, source, source_len);
1917-
if (regs.count > 0) {
1961+
/* A file whose kept list is empty but whose cap still bound is NOT clean:
1962+
* the ranges the cap threw away were never judged by the two rules
1963+
* above, so nothing proves they were recovered. Flag it. */
1964+
if (regs.count > 0 || regs.dropped > 0) {
19181965
result->parse_incomplete = true;
19191966
result->error_region_count = regs.count;
19201967
result->error_ranges = cbm_error_ranges_str(a, &regs);
1968+
/* One range covering nearly the whole file is not advice, it is
1969+
* noise: "look at lines 1 to 13047" of a 13046-line file tells a
1970+
* reader nothing they did not already know. Mark those separately
1971+
* so the report can say "read the source" instead. See
1972+
* parse_unusable in cbm.h for which files land here and why. */
1973+
if (regs.count == 1 && regs.dropped == 0) {
1974+
uint32_t total = cbm_count_lines(source, source_len);
1975+
uint32_t span = regs.ends[0] - regs.starts[0] + 1;
1976+
if (total > 0 && span * 100 >= total * CBM_UNUSABLE_PCT) {
1977+
result->parse_unusable = true;
1978+
}
1979+
}
19211980
}
19221981
}
19231982

internal/cbm/cbm.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,21 @@ typedef struct CBMFileResult {
511511
* completeness guarantee. Callers should treat a flagged file as "prefer
512512
* grep here", never treat an unflagged file as provably complete. */
513513
bool parse_incomplete;
514+
/* True when the ranges cover so much of the file that they are no longer
515+
* useful advice — one range over 80% of the line count. The file WAS
516+
* indexed, but pointing a reader at almost every line tells them nothing,
517+
* so the report says "read the source" instead of listing the range.
518+
*
519+
* Its main customers are non-C languages. The refinement that narrows a
520+
* whole-file range using the preprocessed parse only runs for C, C++ and
521+
* CUDA, so a Python, Java or Ruby file whose root node is ERROR still
522+
* reports 1-N.
523+
*
524+
* Note the naming: this field and the phase string it produces are both
525+
* `parse_unusable`. The older `parse_incomplete` field emits the phase
526+
* `parse_partial` instead. That mismatch is historical, not deliberate —
527+
* do not copy it. */
528+
bool parse_unusable;
514529
const char *error_ranges;
515530
int error_region_count;
516531
bool is_test_file;

0 commit comments

Comments
 (0)