Skip to content

Commit b3ec08d

Browse files
DeusDataDeusData
authored
Merge pull request #2427 from BobbieBarker/fix/registry-test-bit
fix(registry): score a candidate on the extractor's test verdict, not its name
2 parents cf337a6 + cf4f117 commit b3ec08d

11 files changed

Lines changed: 686 additions & 362 deletions

‎src/pipeline/pass_definitions.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -521,13 +521,16 @@ static void process_def(cbm_pipeline_ctx_t *ctx, const CBMDefinition *def, const
521521
* field resolution), Variable/Field (READS/WRITES resolution), and Table/View
522522
* (SQL FROM/JOIN lineage). pass_parallel.c and pipeline_incremental.c seed
523523
* through the same predicate, so the three registries cannot diverge.
524-
* A generated member is registered by qualified name only, at all three. */
524+
* A generated member is registered by qualified name only, at all three.
525+
* The registry's test verdict is the extractor's OR the test-double path
526+
* rule (cbm_registry_test_double_path) on the node's file, at all three. */
525527
if (node_id > 0 && cbm_label_is_registry_symbol(def->label)) {
526528
if (cbm_def_props_generated(def->extra_props)) {
527529
cbm_registry_add_qn_only(ctx->registry, def->qualified_name, def->label);
528530
} else {
529531
cbm_registry_add_lang_in_module(ctx->registry, def->name, def->qualified_name,
530-
def->label, lang, module_prefix_len);
532+
def->label, lang, module_prefix_len,
533+
def->is_test || cbm_registry_test_double_path(def_rel));
531534
}
532535
if (def->qn_sig_off) {
533536
cbm_registry_set_swift_signature(ctx->registry, def->qualified_name,

‎src/pipeline/pass_parallel.c‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1816,22 +1816,25 @@ int cbm_parallel_extract(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t *files,
18161816
/* `file_node_id` is the defining file's node id (0 when it has none), looked up
18171817
* once per file by the caller: computing the file QN and finding its node for
18181818
* every definition was 700 k allocations and lookups on the Go corpus. */
1819-
static int register_and_link_def(cbm_pipeline_ctx_t *ctx, const CBMDefinition *def,
1819+
static int register_and_link_def(cbm_pipeline_ctx_t *ctx, const CBMDefinition *def, const char *rel,
18201820
int64_t file_node_id, CBMLanguage lang, size_t module_prefix_len,
18211821
int *reg_entries) {
18221822
int edges = 0;
18231823
if (!def->name || !def->qualified_name || !def->label) {
18241824
return 0;
18251825
}
18261826
/* Registry membership is defined ONCE by cbm_label_is_registry_symbol
1827-
* (helpers.c) — see pass_definitions.c for the per-label rationale, and
1828-
* for the generated member registered by qualified name only. */
1827+
* (helpers.c) — see pass_definitions.c for the per-label rationale, for
1828+
* the generated member registered by qualified name only, and for the
1829+
* test-double path rule, judged on the path the def's node stores. */
18291830
if (cbm_label_is_registry_symbol(def->label)) {
18301831
if (cbm_def_props_generated(def->extra_props)) {
18311832
cbm_registry_add_qn_only(ctx->registry, def->qualified_name, def->label);
18321833
} else {
1834+
const char *def_rel = def->file_path ? def->file_path : rel;
18331835
cbm_registry_add_lang_in_module(ctx->registry, def->name, def->qualified_name,
1834-
def->label, lang, module_prefix_len);
1836+
def->label, lang, module_prefix_len,
1837+
def->is_test || cbm_registry_test_double_path(def_rel));
18351838
}
18361839
if (def->qn_sig_off) {
18371840
cbm_registry_set_swift_signature(ctx->registry, def->qualified_name,
@@ -2050,7 +2053,7 @@ int cbm_build_registry_from_cache(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t
20502053
: 0;
20512054
for (int d = 0; d < result->defs.count; d++) {
20522055
defines_edges +=
2053-
register_and_link_def(ctx, &result->defs.items[d], file_node_id,
2056+
register_and_link_def(ctx, &result->defs.items[d], rel, file_node_id,
20542057
files[i].language, py_prefix_len, &reg_entries);
20552058
}
20562059
}

‎src/pipeline/pass_tests.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ static const char *itoa_log(int val) {
4545
}
4646

4747
/* Check if a node has is_test:true in its properties JSON. */
48-
static bool node_is_test(const cbm_gbuf_node_t *n) {
48+
bool cbm_node_is_test(const cbm_gbuf_node_t *n) {
4949
if (!n || !n->properties_json) {
5050
return false;
5151
}
@@ -270,13 +270,13 @@ static int create_tests_edges(cbm_pipeline_ctx_t *ctx) {
270270
}
271271

272272
bool src_is_test =
273-
node_is_test(src) || (src->file_path && cbm_is_test_path(src->file_path));
273+
cbm_node_is_test(src) || (src->file_path && cbm_is_test_path(src->file_path));
274274
if (!src_is_test) {
275275
continue;
276276
}
277277

278278
bool tgt_is_test =
279-
node_is_test(tgt) || (tgt->file_path && cbm_is_test_path(tgt->file_path));
279+
cbm_node_is_test(tgt) || (tgt->file_path && cbm_is_test_path(tgt->file_path));
280280
if (tgt_is_test) {
281281
continue;
282282
}

‎src/pipeline/pipeline.h‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -283,9 +283,24 @@ cbm_registry_t *cbm_registry_new(void);
283283
void cbm_registry_free(cbm_registry_t *r);
284284

285285
/* Register a function/method/class. All strings are copied. The entry's
286-
* language is unknown, so name-based resolution never filters it out. */
286+
* language is unknown, so name-based resolution never filters it out.
287+
* `is_test` is the extractor's verdict for this symbol (CBMDefinition.is_test:
288+
* the file's test-ness OR the definition's own test attributes). The registry
289+
* stores it and scores with it; it does not derive test-ness from the QN. */
287290
void cbm_registry_add(cbm_registry_t *r, const char *name, const char *qualified_name,
288-
const char *label);
291+
const char *label, bool is_test);
292+
293+
/* The test-double path rule, for the registry's ranking only: true when the
294+
* repo-relative path of a symbol's defining file has a directory segment named
295+
* exactly `fake`, `mock`, `mocks`, `testing` or `testutil`, or its file name
296+
* matches `mock_*.go` or `fake_*.go`. Go projects keep generated fake
297+
* clientsets and mock packages in ordinary files the extractor rightly does
298+
* not call tests; a call should still prefer real code over them. The three
299+
* registry seeders (pass_definitions.c, pass_parallel.c,
300+
* pipeline_incremental.c) pass the extractor's verdict OR this, so full and
301+
* incremental indexes rank alike. It never decides what a node is: the stored
302+
* is_test property, TESTS edges and importance stay with cbm_is_test_path. */
303+
bool cbm_registry_test_double_path(const char *rel_path);
289304

290305
/* Swift-only metadata and label-compatible overload selection. The returned
291306
* QNs are borrowed from the registry and all belong to one base callable.
@@ -306,7 +321,7 @@ int cbm_registry_swift_candidates_in_file(const cbm_registry_t *r, const CBMCall
306321
* function-local symbols: that needs the defining file's module QN, see
307322
* cbm_registry_add_lang_in_module. */
308323
void cbm_registry_add_lang(cbm_registry_t *r, const char *name, const char *qualified_name,
309-
const char *label, CBMLanguage lang);
324+
const char *label, CBMLanguage lang, bool is_test);
310325

311326
/* As cbm_registry_add_lang. A Python symbol declared inside a function body of
312327
* its own file (cbm_registry_qn_is_function_local, bounded by
@@ -318,7 +333,7 @@ void cbm_registry_add_lang(cbm_registry_t *r, const char *name, const char *qual
318333
* indexes agree. 0 = no local detection. */
319334
void cbm_registry_add_lang_in_module(cbm_registry_t *r, const char *name,
320335
const char *qualified_name, const char *label,
321-
CBMLanguage lang, size_t module_prefix_len);
336+
CBMLanguage lang, size_t module_prefix_len, bool is_test);
322337

323338
/* Register a symbol for exact (qualified-name) lookups only: it is never
324339
* offered to the short-name strategies (unique_name, suffix_match,

‎src/pipeline/pipeline_delta.c‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,28 @@ static char *delta_generated_props(const char *props, const char *generated) {
623623
return out;
624624
}
625625

626+
/* `props` (a JSON object) with the extractor's test verdict added as
627+
* `"is_test":true` (CBM_MEM_CLASS_OTHER; the caller frees), NULL on allocation
628+
* failure. The incremental registry seed reads the verdict back
629+
* (cbm_node_is_test) so a re-resolved caller ranks a test definition below
630+
* real code exactly as the full build does; an absent key reads as false. */
631+
static char *delta_test_props(const char *props) {
632+
static const char member[] = "\"is_test\":true";
633+
size_t pl = props ? strlen(props) : 0;
634+
bool has_members = pl > PAIR_LEN && props[0] == '{' && props[pl - SKIP_ONE] == '}';
635+
size_t cap = pl + sizeof(member) + CBM_SZ_4;
636+
char *out = (char *)cbm_alloc(CBM_MEM_CLASS_OTHER, cap);
637+
if (!out) {
638+
return NULL;
639+
}
640+
if (has_members) {
641+
snprintf(out, cap, "%.*s,%s}", (int)(pl - SKIP_ONE), props, member);
642+
} else {
643+
snprintf(out, cap, "{%s}", member);
644+
}
645+
return out;
646+
}
647+
626648
int64_t cbm_delta_preseed(cbm_store_t *store, const char *project, cbm_gbuf_t *gbuf) {
627649
sqlite3 *db = cbm_store_get_db(store);
628650
if (!db) {
@@ -673,7 +695,13 @@ int64_t cbm_delta_preseed(cbm_store_t *store, const char *project, cbm_gbuf_t *g
673695
* And a generated member's mark (a Java record's implicit accessor, a
674696
* Method node): the registry seed reads it to register the member by
675697
* qualified name only, as the full build does (cbm_node_props_generated);
676-
* without it the proxy came back into the short-name index. */
698+
* without it the proxy came back into the short-name index.
699+
*
700+
* And the extractor's test verdict: the registry seed reads it back
701+
* (cbm_node_is_test) to rank a test definition below real code, as the
702+
* full build does with CBMDefinition.is_test; without it every proxy
703+
* scored as production and a re-resolved caller could bind a test helper
704+
* the full build ranks last. */
677705
sqlite3_stmt *stmt = NULL;
678706
if (sqlite3_prepare_v2(
679707
db,
@@ -702,7 +730,8 @@ int64_t cbm_delta_preseed(cbm_store_t *store, const char *project, cbm_gbuf_t *g
702730
" THEN json_extract(properties, '$.parent_class') END,"
703731
" CASE WHEN label = 'Method'"
704732
" AND instr(properties, '\"" CBM_PROP_GENERATED "\"') > 0"
705-
" THEN json_extract(properties, '$." CBM_PROP_GENERATED "') END"
733+
" THEN json_extract(properties, '$." CBM_PROP_GENERATED "') END,"
734+
" instr(properties, '\"is_test\":true') > 0"
706735
" FROM nodes"
707736
" WHERE project = ?1 AND label NOT IN"
708737
" ('Macro','Comment','Section','Branch','Commit','Tag')"
@@ -727,14 +756,27 @@ int64_t cbm_delta_preseed(cbm_store_t *store, const char *project, cbm_gbuf_t *g
727756
int end_line = sqlite3_column_int(stmt, 7);
728757
const char *parent = (const char *)sqlite3_column_text(stmt, 8);
729758
const char *generated = (const char *)sqlite3_column_text(stmt, 9);
759+
bool is_test = sqlite3_column_int(stmt, 10) != 0;
730760
char *owner_props =
731761
parent ? delta_owner_props(project, name, qn, parent, client_props) : NULL;
732762
const char *props = owner_props ? owner_props : (client_props ? client_props : "{}");
733763
char *generated_props = delta_generated_props(props, generated);
764+
const char *seed_props = generated_props ? generated_props : props;
765+
char *test_props = is_test ? delta_test_props(seed_props) : NULL;
766+
if (is_test && !test_props) {
767+
/* A proxy without its verdict would rank as production code;
768+
* the run cannot be trusted, as with an id mismatch below. */
769+
cbm_free(CBM_MEM_CLASS_OTHER, generated_props);
770+
cbm_free(CBM_MEM_CLASS_OTHER, owner_props);
771+
sqlite3_finalize(stmt);
772+
cbm_log_error("delta.preseed_verdict_alloc_failed", "qn", qn ? qn : "");
773+
return -1;
774+
}
734775
/* Pin the gbuf id to the database id: proxies ARE their rows. */
735776
cbm_gbuf_set_next_id(gbuf, id);
736777
int64_t got = cbm_gbuf_upsert_node(gbuf, label, name, qn, fp ? fp : "", start_line,
737-
end_line, generated_props ? generated_props : props);
778+
end_line, test_props ? test_props : seed_props);
779+
cbm_free(CBM_MEM_CLASS_OTHER, test_props);
738780
cbm_free(CBM_MEM_CLASS_OTHER, generated_props);
739781
cbm_free(CBM_MEM_CLASS_OTHER, owner_props);
740782
if (got != id) {

‎src/pipeline/pipeline_incremental.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1396,8 +1396,11 @@ static void registry_visitor(const cbm_gbuf_node_t *node, void *userdata) {
13961396
size_t prefix_len = lang == CBM_LANG_PYTHON
13971397
? cbm_pipeline_fqn_symbol_prefix_len(seed->project, node->file_path)
13981398
: 0;
1399+
/* The full index's verdict: the extractor's (stored as is_test) OR the
1400+
* test-double path rule on the node's file (pass_definitions.c). */
1401+
bool is_test = cbm_node_is_test(node) || cbm_registry_test_double_path(node->file_path);
13991402
cbm_registry_add_lang_in_module(seed->registry, node->name, node->qualified_name, node->label,
1400-
lang, prefix_len);
1403+
lang, prefix_len, is_test);
14011404
if (node->properties_json) {
14021405
const char *mask = strstr(node->properties_json, "\"param_default_mask\":\"");
14031406
const char *count = strstr(node->properties_json, "\"selector_param_count\":");

‎src/pipeline/pipeline_internal.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,12 @@ static inline bool cbm_pipeline_test_force_def_modules_null(const cbm_pipeline_c
657657
typedef bool (*cbm_result_want_fn)(const CBMFileResult *header);
658658
CBMFileResult *cbm_pipeline_result_acquire(const cbm_pipeline_ctx_t *ctx, CBMFileResult **cache,
659659
int i, cbm_result_want_fn want, bool *loaded);
660+
/* True when a graph-buffer node carries is_test:true in its properties JSON.
661+
* The incremental registry seed has only the persisted node, not the
662+
* CBMDefinition the definition passes hand to cbm_registry_add, and both must
663+
* give the registry the same verdict for the same symbol. */
664+
bool cbm_node_is_test(const cbm_gbuf_node_t *n);
665+
660666
void cbm_pipeline_result_release(CBMFileResult *r, bool loaded);
661667

662668
/* Log the store counters, close and delete the store, drop the latch. */

0 commit comments

Comments
 (0)