fix(store): route label allowlists through the canonical type-like set - #1380
Conversation
cbm.h documents cbm_label_is_type_like() as the single source of truth for
type-like labels, "so adding a new type-like label (e.g. Struct for
Rust/Go/Swift/D structs) updates them all at once instead of scattering
|| strcmp(label,"Struct")==0 across the tree".
A SQL string literal cannot call it. Four queries in store.c and the BM25
ranking in mcp.c carried their own hardcoded label lists, so they silently
opted out of that contract and stopped matching the moment Struct, Interface,
Enum, Type and Trait began being emitted:
store.c:5279 arch_boundaries ('Function','Method','Class')
store.c:5422 arch_packages_from_qn ('Function','Method','Class')
store.c:6689 arch_clusters ('Function','Method','Class')
store.c:7742 cbm_store_vector_search ('Function','Method','Class')
mcp.c:2759 BM25 ranking ('Class','Interface','Type','Enum')
Effect on main today: get_architecture (boundaries, packages, clusters) and
vector search drop every Struct, Interface, Enum, Type and Trait node in the
project, and search_code under-ranks structs -- for Rust, Go, Swift and D,
where struct is the primary type declaration. The BM25 list had drifted
differently again, omitting Struct and Trait but not Interface.
The fix is not a wider literal. CBM_SQL_TYPE_LIKE_LABELS and
CBM_SQL_CALLABLE_OR_TYPE_LABELS live next to each other in constants.h and are
pinned to cbm_label_is_type_like() by a test that checks both directions:
every label the C predicate accepts appears in the SQL fragment, and nothing it
rejects is smuggled in. Adding a type-like label without updating the SQL now
fails that test instead of quietly shrinking query results.
No new labels are introduced. Every label added here is one the extractors
already emit.
Reported by @win4r in #568, whose audit found four of the five sites. Their
patch widened the literals in place and added Actor, a label main does not
emit; this routes through the canonical set instead and also covers Interface,
Type and Trait, which that patch still omitted.
Co-Authored-By: win4r <win4r@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
test-runner, test-repro-runner, test-runner-tsan, codebase-memory-mcp and test-foundation are each built in a single compiler invocation from their sources, and their rules listed only .c files as prerequisites. Editing a header therefore changed nothing make could see, so the target was not rebuilt and an incremental build silently kept testing the OLD header value. Found while revert-checking the label-allowlist change in this branch: the check passed, which was wrong. Shrinking CBM_SQL_TYPE_LIKE_LABELS should have turned the pin test red and did not, because nothing had recompiled. Forcing a rebuild produced the expected failure. A revert-check that cannot fail is worse than no revert-check, because it reports confidence it did not earn. This is the same class as the lsp_all.o staleness that #662 fixed with an explicit dep list; that fix was per-object, this one covers the whole-program targets. PROJECT_HDRS globs src/, internal/cbm/ and tests/ headers and is added to the five affected targets. Vendored trees are deliberately excluded: they are pinned, they already carry explicit dep lists (LSP_UNITY_DEPS, TS_RUNTIME_DEPS), and globbing tens of thousands of grammar headers would cost more than it protects. Verified: editing only src/foundation/constants.h, with no source file touched, now rebuilds and turns the pin test red -- the exact edit that was invisible to make before. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
|
Added a second commit: the build now rebuilds when a project header changes. This is the gap the PR description mentioned as "being reported on its own" — it turned out to belong here, because it is what made the first revert-check of this change lie.
That is exactly what happened here: shrinking Same class as the
Verified the fix directly: editing only Re-ran after restoring: 551 passed, 2 skipped across |
Distilled from #568 by @win4r, with credit on the commit.
The bug
cbm.hdocumentscbm_label_is_type_like()as the single source of truth for type-like labels — "so adding a new type-like label (e.g.Structfor Rust/Go/Swift/D structs) updates them all at once instead of scattering|| strcmp(label,"Struct")==0across the tree."A SQL string literal cannot call it. Five queries carried their own hardcoded label lists and silently opted out of that contract:
store.c:5279arch_boundaries('Function','Method','Class')store.c:5422arch_packages_from_qn('Function','Method','Class')store.c:6689arch_clusters('Function','Method','Class')store.c:7742cbm_store_vector_search('Function','Method','Class')mcp.c:2759BM25 ranking('Class','Interface','Type','Enum')On main today:
get_architecture(boundaries, packages, clusters) and vector search drop everyStruct,Interface,Enum,TypeandTraitnode in the project, andsearch_codeunder-ranks structs. For Rust, Go, Swift and D — wherestructis the primary type declaration — that means the architecture view omits most of the codebase's types.Note the BM25 list had drifted differently again: it includes
Interfacebut omitsStructandTrait. Five hand-maintained lists, three different answers.The fix
Not a wider literal — that would just reset the same trap.
CBM_SQL_TYPE_LIKE_LABELSandCBM_SQL_CALLABLE_OR_TYPE_LABELSlive inconstants.h(the one header bothstore.candmcp.calready include) and are pinned to the C predicate by a test that checks both directions:cbm_label_is_type_like()accepts appears in the SQL fragment, andAdding a type-like label without updating the SQL now fails that test instead of quietly shrinking query results.
No new labels are introduced. Every label added here is one the extractors already emit today.
Verification
store_nodes,mcp,store_search— 314 passed, 2 skippedmake -f Makefile.cbm lint-ci— cleanCBM_SQL_TYPE_LIKE_LABELSback to'Class'fails the pin attest_store_nodes.c:36. Worth noting how that was confirmed — the first revert-check passed spuriously becauseconstants.his not tracked as a build dependency, so nothing rebuilt. Forcing a rebuild produced the expected RED. That header-dependency gap is a separate issue and is being reported on its own.Credit
@win4r's audit in #568 found four of the five sites — the finding is theirs. Their patch widened the literals in place and added
Actor, a label main does not emit; this routes through the canonical set instead, and also coversInterface,TypeandTrait, which their patch still omitted.