Skip to content

M2 idiomatic kinds - #568

Closed
win4r wants to merge 15 commits into
DeusData:mainfrom
win4r:m2-idiomatic-kinds
Closed

M2 idiomatic kinds#568
win4r wants to merge 15 commits into
DeusData:mainfrom
win4r:m2-idiomatic-kinds

Conversation

@win4r

@win4r win4r commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

KerseyFabrications and others added 15 commits June 20, 2026 23:09
A node group variable carried through a WITH aggregation
(e.g. `WITH g, count(*) AS c RETURN g.file_path`) returned blank for every
property except its name: the carried virtual binding held only the group
key (the node's name) and lacked a store handle, so node_prop() could
neither read other fields nor compute degrees.
Fix: capture the node id of a bare node group-var in with_agg_find_or_create
and tag the carried virtual binding with it; in node_prop(), when such a stub
(id set, string fields unpopulated) is asked for a missing property, re-fetch
the full node via cbm_store_find_node_by_id and project it. Also propagate
the store onto virtual bindings so node_prop can re-fetch and compute
degrees. The stub gate is heuristic but never yields a wrong value — worst
case is one redundant indexed lookup. Adds regression test
cypher_exec_with_node_groupvar_prop.

Signed-off-by: Kris Kersey <kris@kerseyfabrications.com>
(cherry picked from commit 8b03974)
…esults

MATCH (c:Class)-[:DEFINES_METHOD]->(m:Method) returned at most 10 results
for any class, regardless of how many methods it actually has.

Root cause: bind_cap was set to scan_count (the number of nodes matched in
the initial pattern — typically 1 when querying a single class by name).
max_new = bind_cap * 10 = 10, so the edge expansion loop exited after
collecting 10 results. No error, no warning, no truncation indicator.

This is language-agnostic: any class with more than 10 methods in any
language was silently truncated. The fix is two characters:
  bind_cap = scan_count > max_rows ? scan_count : max_rows

Regression test: a Python class with 15 methods must return all 15 via
MATCH (c:Class)-[:DEFINES_METHOD]->(m:Method) with label filtering.

Signed-off-by: Thomas Dyar <tdyar@intersystems.com>
(cherry picked from commit c43fc8d)
A call carrying enough long arguments drove append_args_json()'s running
position past the fixed CBM_SZ_2K `props` stack buffer in
emit_normal_calls_edge(): format_call_arg() returns snprintf's *untruncated*
length, so `pos += (size_t)n` could exceed `bufsize`, after which the
trailing `buf[pos] = '\0'` (and `buf[pos++] = ']'`) wrote out of bounds. The
stack canary caught it as SIGABRT, so full-repo indexing of large TypeScript
codebases crashed the server in the parallel resolve pass
(emit_service_edge -> emit_normal_calls_edge -> finalize_and_emit ->
append_args_json). Confirmed with AddressSanitizer:
stack-buffer-overflow WRITE at pass_parallel.c:1124, 'props' (2048 B).

Fix: when an argument does not fully fit, roll back to before its separator
and stop appending (atomic field, matching append_json_string's behaviour),
so `pos` can never advance past the buffer.

Add regression test parallel_args_json_no_overflow: indexes a fixture whose
single call carries 60 long string args (args JSON well past 2 KB); under the
ASan test build it aborts without this fix and passes with it.

Signed-off-by: Andrius Skerla <1492322+rainder@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 74d15a6)
Signed-off-by: Saurav Kumar <sauravsk2507@gmail.com>
(cherry picked from commit c3a1a79)
git_allocator moved out of the top-level git2.h into git2/sys/alloc.h
in libgit2 1.8.0. Add an explicit include so the mimalloc binding
compiles against libgit2 >= 1.8 (e.g. MacPorts libgit2 1.9.4).

(cherry picked from commit 586fc8a)
manage_adr stores ADRs in project_summaries, but a full re-index
(triggered by file changes or new files) deletes the DB in
try_incremental_or_delete_db and rebuilds it from the graph buffer,
which writes an empty project_summaries table. file_hashes were
re-persisted after the rebuild but project_summaries were not, so the
ADR was silently lost.

Fix: capture the ADR before the DB is unlinked, stash it on the
pipeline struct, and restore it after the rebuilt DB is reopened in
dump_and_persist_hashes. The incremental path is unaffected (it never
rewrites the DB). Verified: ADR now survives a full re-index.

Signed-off-by: RithvikReddy0-0 <rithvikreddymukkara@gmail.com>
(cherry picked from commit 7b6c063)
detect_changes advertised a `since` parameter in its inputSchema but the
handler never read it — it always diffed against base_branch (default
"main"), so detect_changes(since="HEAD~10") silently returned the wrong or
empty result when HEAD was on the default branch.

Fix: read `since` and, when present, route it through base_branch so the
existing shell-arg validation (cbm_validate_shell_arg) and the
`<base>...HEAD` diff apply unchanged; `since` takes precedence over
base_branch. Also narrows the schema description — the prior "date" form
(e.g. 2026-01-01) is not a revision and never worked through this path — and
documents the inherited three-dot semantics. Adds regression tests
tool_detect_changes_since and tool_detect_changes_since_precedence.

Refs DeusData#371

Signed-off-by: Kris Kersey <kris@kerseyfabrications.com>
(cherry picked from commit 53501b0)
trace_path resolved a function_name from the first row of an unordered name
query with no ambiguity check, so a same-named entity (e.g. a shell script's
main()) could silently shadow the intended C main(). get_code_snippet
reported "ambiguous" for a short name even when one match was the obvious
definition (the .c body vs a .h declaration).

Fix: add a deterministic resolution ranking — a callable label outranks a
module, then the larger definition by line span wins, preferring a real
definition without hardcoding file extensions — and a picker that flags a
genuine tie. trace_path now traces the preferred node and returns the
existing ambiguous-suggestions response on a true tie instead of silently
taking nodes[0]; get_code_snippet resolves directly to the preferred match,
reporting ambiguity only for real ties. Adds regression tests
tool_trace_call_path_ambiguous and tool_trace_call_path_prefers_definition.

Signed-off-by: Kris Kersey <kris@kerseyfabrications.com>
(cherry picked from commit 382dc24)
Signed-off-by: King Star <mcxin.y@gmail.com>
(cherry picked from commit 935027a)
Mark this as a community fork of DeusData/codebase-memory-mcp (MIT, © 2025
DeusData) and list the integrated incremental-reindex fix (DeusData#528) plus the
9 cherry-picked upstream PRs (DeusData#465 DeusData#412 DeusData#475 DeusData#527 DeusData#512 DeusData#539 DeusData#464 DeusData#466 DeusData#526).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: win4r <win4r@outlook.com>
Re-runnable deterministic comparison (dup-nodes, kinds, call-graph parity).
Baseline on LingoLearn: cbm dup_nodes=38, Swift-type-kinds=1 vs codegraph 0/5.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…unction

push_class_body_children's body-container list had drifted from extract_class_def's
and lacked enum_class_body/protocol_body, so Swift enum/protocol members were
re-walked and emitted as spurious top-level Functions (38 dup-nodes on LingoLearn).
Route those bodies through the nested-class path. dup_nodes 38->0; real Methods +
their CALLS edges unchanged (review keeps 7 callers). Adds regression test.

WS2a of the M1 'surpass codegraph' track. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…own)

WS1 of the M1 'surpass codegraph' track. New `explore` MCP tool composes the
existing resolve / cbm_store_bfs / resolve_snippet_source / batch_count_degrees
internals into ONE agent-ergonomic call returning markdown: blast-radius
(attributed callers) + verbatim line-numbered source grouped by file, with
inline fan-in hotspot flags and a query_graph (cypher) escape-hatch footer.
Matches codegraph's explore and exceeds it (precise caller attribution +
hotspots + cypher, which codegraph's explore lacks).

Adversarially reviewed (5 lenses, each finding refuted against the code);
memory-safety clean. Fixed all 3 confirmed honesty/silent-truncation findings:
clamp depth>=1 + honest 'within N hops' label for depth>1; elision marker when
a body exceeds 160 lines; cap notice when >16 query terms. Adds tests:
explore-in-tools/list (schema validity) + 2 error-path guards.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…odegraph 79)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t from class

WS2b of the M2 track. tree-sitter-swift emits one class_declaration node for
class/struct/enum/actor (distinguished by the declaration_kind keyword field);
relabel to Struct/Enum/Actor (class stays Class, protocol already Interface) so
the graph distinguishes Swift type kinds — closes codegraph's modeling edge
(LingoLearn: 1 lumped kind -> Struct:38/Enum:20/Class:6, Swift-kind-fidelity 1->3).

Label is load-bearing: added Struct/Enum/Actor to every resolver allowlist
(registry x3, resolve_as_class x2, store.c arch/semantic SQL x4, pass_configlink
CONFIGURES, pass_enrichment decorators + nlabels, search_code ranking) so
CALLS/INHERITS/USES_TYPE/CONFIGURES edges + architecture/search are unaffected
for real user code (review keeps 7 callers, extension-method callers intact:
addingDays 11, tap 12; dup_nodes 0).

Adversarially reviewed (4 lenses, 12 agents). Fixed a HIGH bug: a same-file
`extension` shares the extended type's FQN, so its (Class) type def clobbered the
real type's idiomatic label via the last-write-wins upsert (struct X -> Class).
Extensions now extract members but emit NO type def — which also removes the
phantom 'Class' nodes previously created for stdlib types the code only extends
(Date/Color/View). Net effect: total edges 1813->1689 (dropped edges are spurious
stdlib-constructor CALLS to those phantom nodes, NOT user-code relationships;
brings cbm closer to codegraph, which also doesn't node merely-extended stdlib
types). Adds 2 regression tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@DeusData

Copy link
Copy Markdown
Owner

Thanks @win4r — this is the cumulative branch (it includes #567 + #569 plus the Swift idiomatic type kinds Struct/Enum/Actor, which are a nice addition).

Same blockers as the other two: the README rebrand to a personal fork, the bench/ harness with a hardcoded personal path, the empty description, and DCO not signed.

Rather than three stacked, fork-branded PRs, could you send the useful pieces as focused, signed-off PRs — the explore tool, the Swift fixes/kinds — dropping the README rebrand and bench/? We'd be glad to land those — the engineering is genuinely good; it just needs to arrive clean (no rebrand/bench) and with CI green (signed-off commits, lint and tests passing). 🙏

@DeusData DeusData added enhancement New feature or request cypher Cypher query language parser/executor bugs parsing/quality Graph extraction bugs, false positives, missing edges priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Jun 29, 2026
@DeusData DeusData added this to the 0.9.2-rc milestone Jul 8, 2026
@DeusData

Copy link
Copy Markdown
Owner

Reviewed as the top of the stack — these three are strict containment (#567#569#568), so this PR carries everything in both others. I have closed #567 (fully superseded) and left detailed notes on #569; please read those first. This covers what is unique to #568.

Your extension-clobber catch is sharp, and it may well be a live bug on main.

You spotted that a same-file extension X re-labels struct X back to Class, because the extension pushes a type def with the same qualified name and the UNIQUE(project, qualified_name) upsert makes it last-write-wins. Looking at current main: it still pushes a type def for extension, and since declaration_kind is not struct the label stays Class — so the same clobber mechanism appears to be present. That is a real graph-quality bug and worth its own PR with a repro.

Similarly, main only special-cases Swift struct. enum and actor still lump to Class, so your swift_idiomatic_type_kinds test would be red on main for those two. Genuine repro material.

But the mechanism needs rebuilding before it can land. This PR extends roughly ten hardcoded label allowlists — registry gates in pass_definitions, pass_parallel, pipeline_incremental, both resolve_as_class sites, four store SQL lists, pass_configlink, pass_enrichment, BM25 ranking. Since you branched, main centralised exactly this into cbm_label_is_type_like() (helpers.c:160), whose comment says it plainly: "Single source of truth … adding a label here updates them all." That helper exists precisely to kill the drift that scattered allowlists cause, so re-adding ten of them would move us backwards. A rebase here is a reimplementation, not a conflict resolution.

And one piece is a taxonomy decision rather than a fix. Actor would be a new first-class label in the graph schema — it is not in cbm_label_is_type_like() today. Adding a label has blast radius across every consumer, so that is the maintainer's call and I have escalated it alongside the Swift Enum question. Your instinct matches our stated doctrine (production emits the precise label, consumers get updated rather than the test relaxed) — it is the execution that has aged, not the direction.

Something your work surfaced about main itself, which I am filing regardless of these PRs: store.c still uses label IN ('Function','Method','Class') at four sites (arch boundaries, clusters, vector search). Main already emits Struct nodes for Rust, Go, Swift and D — and those nodes are being excluded from all four today. That is a bug on main that your allowlist audit exposed, and it would have gone unnoticed otherwise. Thank you for that.

Practical asks for any resubmission, all of which currently block CI regardless of merit: 4cd82fa has no Signed-off-by, so DCO fails; the AI session Co-Authored-By trailers should be stripped; and one concern per PR.

One thing that needs independent verification if this concept is revived: the commit itself notes total edges dropping 1813 → 1689 and claims all of them were spurious. That is plausible given the clobber fix, but an edge-count drop is exactly the kind of claim we want measured against a current main rather than inherited.

Leaving this open pending the maintainer's taxonomy answer, rather than closing your work while a decision is outstanding.

@DeusData

Copy link
Copy Markdown
Owner

Closing this, and I want to be clear that part of your work has landed on main rather than letting a close read as a rejection.

What shipped, with your credit

#1380 (63eda54, merged as 880b87a6a) carries Co-Authored-By: win4r. Your allowlist audit found that four SQL queries in store.c and the BM25 ranking in mcp.c hardcoded their own label lists, so get_architecture (boundaries, packages, clusters) and vector search were dropping every Struct, Interface, Enum, Type and Trait node in the project — which, for Rust, Go, Swift and D, means most of the codebase's types were missing from the architecture view. That was live on main and nobody else had spotted it.

Two changes from your version, both deliberate. It routes through the canonical cbm_label_is_type_like() set rather than widening the literals in place, and it is pinned by a test in both directions, so the next type-like label fails CI instead of quietly shrinking query results. And it does not include Actor, since main does not emit that label — including it would have been shipping new vocabulary through a bug fix.

Your audit also surfaced a second problem indirectly: while revert-checking that fix, the check passed when it should have failed, because the build did not rebuild on a header change. That is fixed in the same PR.

Why the PRs themselves cannot land

Neither is mergeable in its current shape: both are CONFLICTING against a main that has moved a long way, dco and lint are red, and #568 contains #569 with identical commit OIDs — so merging #568 would silently also take the README rebrand to codebase-memory-mcp-pro and the bench/ harness, which hardcodes a local path and shells out to an unpinned external binary. Those two cannot come upstream in any form.

The nine cherry-picked commits are all already on main under different SHAs, so they are now pure conflict noise.

What is recorded, and still owed to you

Two further bugs you found are real, verified, and on the maintainer's list — they are not being dropped by this close:

  1. Swift enum and protocol members are double-emitted. push_class_body_children scans body containers by child type and omits enum_class_body and protocol_body, while find_class_body locates them via the body field — which a child-type scan structurally cannot see. Every Swift enum static and protocol member is emitted twice, once as a Method and once as a spurious top-level Function. Your measurement of 38 duplicate nodes on a 29-file repo checks out, and your fix auto-merges cleanly onto current main.
  2. Swift extension clobbers the type it extends. Because Swift qualified names bake in the file stem, struct P {} and extension P {} in P.swift both compute the same QN, and last-write-wins relabels P back to Class and relocates its file_path and line range onto the extension — so get_code_snippet points at the wrong code. Cross-file, extension Date {} mints a phantom Class node for a stdlib type.

The first is held only because it touches the same function as an open architecture decision on another PR — we do not want to answer the same prevention-versus-suppression question twice, in the same function, with different reasoning. When that lands it will carry your credit too.

Left open for the maintainer

The Enum and Actor labelling and the explore tool are direction calls, not defects. Enum is low-risk since it already exists in the canonical set; Actor would be a genuinely new permanent label that every type-resolution consumer would silently drop unless added there as well.

Thank you

Four real defects on main, one of which surfaced only because you audited allowlists nobody else thought to check, and another you reasoned out correctly from the UNIQUE upsert semantics. Your tests pin behaviour rather than asserting it. That is a better hit rate than most review rounds produce, and it is why one of these is already shipped and two more are queued rather than filed away.

@DeusData DeusData closed this Jul 31, 2026
pull Bot pushed a commit to MrDolphin/codebase-memory-mcp that referenced this pull request Jul 31, 2026
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 DeusData#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cypher Cypher query language parser/executor bugs enhancement New feature or request parsing/quality Graph extraction bugs, false positives, missing edges priority/normal Standard review queue; useful PR with ordinary maintainer urgency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants