A pass over places where a bound was either miscalculated or missing.
UI response builders (src/ui/http_server.c) mixed the clamping helper
http_appendf with raw cursor indexing. Route the per-line separators and
quotes through the helper too, so the cursor cannot leave the buffer once
output saturates it, and terminate explicitly afterwards — the helper pins
the cursor and then writes nothing, which would otherwise leave the "%s"
reply with no terminator in range. The log buffer was budgeted at
LOG_LINE_MAX + 10 per line while JSON escaping can double every byte, so
size it for the escaped worst case. Job status entries now escape their path
and error fields, which were interpolated raw.
Cypher (src/cypher/cypher.c): the WHERE grammar descends once per nested '('
and once per NOT, so parse depth followed the query text rather than
anything bounded. Track depth on parser_t and refuse past 256 levels with a
parse error. Bounding parse depth also bounds the resulting tree, so the
recursive evaluator inherits the limit.
Gitignore (src/discover/gitignore.c): '**' retries the remainder at every
position and consecutive groups multiply, so match cost is exponential in
the number of groups. Thread a step budget through the matcher and give up
past 20000 steps, reporting no-match so a pathological pattern fails to
ignore rather than ignoring the wrong files. glob_match keeps its name so
recursion_whitelist.h still describes the functions that recurse; the new
non-recursive wrapper that seeds the budget is glob_match_bounded.
Call extraction (internal/cbm/extract_calls.c): extract_fp_callee recursed
once per applied argument, so stack use followed the parse-tree depth of the
indexed file. Rewritten as a left-spine loop. Reassigning node/nk before
continuing leaves the fall-through cases where the recursive form left them,
so behaviour is unchanged and the extraction suite is untouched.
CALLS props (src/pipeline/pass_calls.c): `cap - pos - PAIR_LEN` wraps once
pos reaches cap - PAIR_LEN and stops bounding the copy below. Guard
additively, matching the closing write in the same function.
Shell-bound paths (src/foundation/str_util.*): the three git shell-out sites
each wrap a repo path in cmd.exe-compatible double quotes, where %VAR%,
!VAR! and ^ remain active. Two carried private copies of the check that
rejects those three on Windows and the third used the bare validator.
Promote the stricter form to cbm_validate_shell_path_arg and route all three
through it, so the copies cannot drift apart again.
Tests cover the buffer bounds, the parse-depth refusal, matcher termination
and the validator. The bounds tests run in a forked child so a violation
surfaces as a signal instead of silently.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The Cypher request model stored a single order_by expression; on
ORDER BY key1 DESC, key2 ASC LIMIT n the parser consumed only key1,
left ', key2 ... LIMIT n' unparsed, and the query silently returned the
entire result set (6326 rows / 117 KB instead of 5 on the reporter's
graph - a token flood straight into agent context).
The return clause now models up to CBM_CYPHER_ORDER_KEYS_MAX (8) sort
keys with per-key direction (Cypher semantics). Both sort sites -
rb_apply_order_by for RETURN and sort_bindings for the WITH pipeline -
compare key-by-key with later keys breaking ties. More keys than the
modeled maximum is a loud parse error, never a silently dropped
remainder. The no-ORDER-BY projection fast path is preserved via
order_key_count == 0.
Tests: parse-level (2 keys, per-key direction, LIMIT consumed;
9-key over-cap rejected), exec-level RETURN (limit kept, tiebreak
ordering, per-key direction), and exec-level WITH pipeline (limit kept).
All RED before the fix (row_count 4 instead of 2), GREEN after.
Recorded, pre-existing and unchanged: an ORDER BY key that is not part
of the projection is silently skipped (single-key main behaves the
same); sorting by unprojected properties needs hidden-column projection
and is a separate issue.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
process_edges and expand_var_length carried the expansion budget in the LOOP
condition (`ei < edge_count && *new_count < max_new`). Once new_count reached
max_new they stopped iterating entirely, so match_count was never incremented
for the remaining sources -- even though those sources had real neighbours.
expand_pattern_rels' OPTIONAL fallback is ungated (it has a reserved slot per
binding since the #1177 sizing fix), so it saw match_count == 0 and emitted an
unbound row for every such source.
The result is a false negative that reads as a positive assertion:
MATCH (f:Function) OPTIONAL MATCH (f)-[:CALLS]->(c) WHERE c IS NULL
reports a function as having no callers when it has callers, once an earlier
function in the same scan saturated the budget. That is a dead-code query
telling a user that live code is dead.
The budget caps MATERIALISATION, never detection. Both loops now iterate the
full candidate set, increment match_count whenever a real neighbour passes the
filters, and gate only the write into new_bindings. The bound proof from #1177
is unchanged -- writes are still capped at max_new and fallbacks are still at
most one per binding -- so allocation safety does not depend on this change.
Cost: a saturated source still performs its node lookups and filter checks, the
same ones the unsaturated path performs, and materialises nothing.
Reported by @SEPURI-SAI-KRISHNA while working on the sibling bound-terminal path
in #1378; this is the same defect in expand_pattern_rels, which was already live
on main rather than introduced there.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The bare-node-carry check in with_agg_find_or_create only tested
!property && variable, so labels(n)/id(n)/keys(n)/properties(n) aliases
(variable set, property NULL, func set) were also tagged with the source
node's id. A later alias.property then hit node_prop's stub re-fetch
heuristic and silently returned the source node's real property instead
of empty for the non-node alias.
Adds cypher_issue1111_with_scalar_func_alias_no_node_leak, which fails
on the prior code (returns the source node's file_path) and passes with
this fix.
Addresses DeusData's review on #1221.
Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
RETURN/WITH aggregation classified each projected column with a bare
`if (item->func)` check to decide group-key vs. aggregate value. That
check is true for any function call, so type()/labels()/id()/keys()/
properties() were routed into the aggregate branch alongside real
aggregates and formatted via format_agg_value's default case, which
emits the row count instead of the function's actual value.
Swap the five call sites (ret_agg_build_key, ret_agg_emit_row,
with_agg_build_key, with_agg_find_or_create, with_agg_accumulate) to
is_aggregate_func(), the predicate already used correctly one level up
to decide whether a query needs aggregation at all. Group-key columns
carrying a non-aggregate function now project through the existing
project_item() helper instead of binding_get_virtual(), since they
need to evaluate the function rather than read a raw property.
Fixes#1111 (the type(r)/count(*) half; the inline-property-map half
of that issue does not reproduce on current HEAD)
Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
An unbounded whole-graph OPTIONAL MATCH or a GROUP BY that produces roughly one
group per node does O(bindings x groups) work in execute_return_agg and can run
for minutes on a large graph before emitting a single row. The 100k row ceiling
never fires (no rows are produced), so query_graph just hangs with no partial
result and no error (#601).
Arm a monotonic wall-clock budget (default 30s) at query entry and check it
(throttled, every 1024 iterations) in the scan, relationship-expansion and
aggregation hot loops. On expiry the query aborts with a clear, actionable error
instead of hanging. A thread-local test hook forces the budget so the guard is
covered by a deterministic reproduce-first test; a companion test confirms the
default budget does not false-positive on a normal query.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
json_extract_prop() scanned a non-string property up to the first ',' and a
string property up to the first '"', ignoring nesting and backslash escapes.
Any array/object property was therefore truncated at its first INTERNAL comma,
and any string containing an escaped quote was cut short.
decorators: ["@Roles('OWNER', 'ADMIN')","@Get()"]
projected as: ["@Roles('OWNER'
This makes decorator/route/authz queries unusable on frameworks whose
decorators take multiple arguments (NestJS, Angular, Spring). Values are now
copied as balanced constructs, honoring string state and escape pairs; scalar
and plain-string paths are unchanged.
Signed-off-by: KolisCode <jhohantma@gmail.com>
Three of #797's four findings fixed (the fourth — repeated-variable
unification — already holds on main and is now pinned by the test):
- Walk padding: the BFS recursive CTE deduped (node, hop) PAIRS, so a
single self-loop minted every hop level for every node it could
reach — *k..k matched fabricated walks of arbitrary length (a
100-hop 'longest chain' artifact) and the row set exploded to
nodes x depth (the reported crash). The traversal now returns each
node once at its MINIMAL hop (MIN/GROUP BY): variable-length matches
nodes whose shortest path lies in [min,max]. This is deliberately
shortest-path semantics, not openCypher trail enumeration — right
for reachability/depth audits on call graphs and linear instead of
exponential; trace_path shares the traversal and now also reports
each node once on cyclic graphs.
- Silent 100-hop cap: a clamped hop range now surfaces as
result->warning ('clamped ... an empty result may mean clamped, not
no such path'), emitted by query_graph in both TOON and JSON forms,
on top of the existing log WARN.
Reproduce-first: cypher_exec_varlength_path_semantics_issue797 —
self-loop + real 2-chain fixture; RED on the walk semantics (*2..2
returned 3 endpoints instead of 1), GREEN now, incl. variable
unification, zero rows for nonexistent path lengths, and the clamp
warning assertion.
Closes#797
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The WHERE parser rejected coalesce() outright ('unexpected operator'),
blocking null-safe numeric filters over optional graph properties —
the exact audit-query shape from the report:
MATCH (f:Function) WHERE coalesce(f.transitive_loop_depth, 0) >= 2
RETURN-side coalesce already worked; only the WHERE leaf lacked it.
Implementation: the condition leaf gains a coalesce_default literal.
The operator/value tail of condition parsing is factored into a shared
parse_condition_op so both subjects (var[.prop] and
coalesce(var.prop, literal)) accept the full operator set (comparisons,
IS [NOT] NULL, IN). At eval time a missing/empty property value is
substituted with the default before the operator runs. Supported form
is coalesce(var.prop, literal) — matching openCypher's two-argument
null-substitution for this subset.
Reproduce-first: cypher_exec_where_coalesce_issue874 runs the
reporter's exact query — RED (rc=-1 parse error) on the old parser —
and pins both directions: a failing default excludes property-less
nodes, a passing default includes them.
Closes#874
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The result projection is materialized per row into fixed-width stack
arrays (CBM_SZ_32 columns) in execute_return_simple and its siblings,
indexed by the parsed RETURN item count — which the parser left
unbounded. A RETURN/WITH clause with more than 32 projection items
wrote past those arrays. Reject an over-wide projection at parse time,
with a fork-isolated regression test driving a 48-column RETURN.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Explicit variable-length Cypher bounds ([:CALLS*1..N]) and the client
`depth` on trace_call_path / detect_changes were passed straight into
cbm_store_bfs, so an arbitrarily large value drove an unbounded traversal
(a DoS on cyclic graphs). The MCP_MAX_DEPTH ceiling was defined but never
applied, and CYP_MAX_DEPTH capped only the unbounded var-length forms.
Add env-configurable ceilings cbm_cypher_max_depth() (CBM_CYPHER_MAX_DEPTH,
default 10) and cbm_mcp_max_depth() (CBM_MCP_MAX_DEPTH, default 15), clamp
both entry points to them, and WARN on clamp -- never a silent truncation.
Remove the now-dead CYP_MAX_DEPTH / MCP_MAX_DEPTH constants.
Reproduce-first: cypher_exec_var_length_explicit_bound_capped (12 -> 10 rows)
and tool_trace_call_path_depth_clamped (depth 1000 -> hop 15).
Closes#887.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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>
Per-function complexity metadata is now stored on graph nodes and queryable,
alongside several indexing performance and correctness fixes developed and
validated together (3704 tests, ASan/UBSan clean).
Bottleneck metrics (query via query_graph):
- Tier A (in the extraction AST walk): cyclomatic (complexity), cognitive
(nesting-weighted), loop_count, loop_depth (max nested-loop depth),
param_count, max_access_depth.
- Tier B (new pre-dump pass, pass_complexity.c): transitive_loop_depth
propagated along CALLS edges + a recursive flag (direct self-recursion and
mutual-recursion cycles), plus the call-context signals linear_scan_in_loop,
alloc_in_loop, recursion_in_loop and unguarded_recursion.
- query_graph and get_architecture tool descriptions document the metrics and
the Leiden community clusters.
Cypher engine:
- node_prop exposes arbitrary persisted node properties to WHERE/RETURN.
- Fix projection aliasing: multi-property rows shared a single static buffer so
every column returned the last value read; now per-column/rotating buffers.
- Fix a stack-use-after-scope in aggregate RETURN (caller-owned value buffers).
Indexing performance:
- Gate C/C++ #define Macro-node extraction to full mode (it is ~49% of nodes on
the Linux kernel); moderate/fast skip it.
- Emit the complexity property block only for Function/Method nodes so the
millions of Macro/Field/Variable/Class/Enum nodes no longer carry zeroed
fields — large RAM reduction at scale.
- Classify node types via tree-sitter TSSymbol bitsets in cbm_kind_in_set
instead of per-node strcmp scans (thread-local cache, strcmp fallback;
behaviour-identical).
- Subsample frequent (Zipfian) tokens in the semantic co-occurrence finalize;
~14x faster finalize on the kernel, output unchanged.
- pass_lsp_cross: replace O(n^2) linear dedup with hash-set dedup.
Windows:
- Canonicalize drive-letter case during path normalization so "c:/repo" and
"C:/repo" derive the same project key and cache file (#394/#227/#367).
Tests: extraction, pipeline and cypher regressions covering all of the above.
Add a single-hop, anchored existence predicate:
WHERE NOT EXISTS { (f)<-[:CALLS]-() } -- functions with no caller
WHERE EXISTS { (f)-[:CALLS]->() } -- functions that call something
Parsed via parse_exists_predicate (reusing parse_node/parse_rel) into a leaf
condition (op=EXISTS, anchor variable, edge type, direction); evaluated against
the bound node with cbm_store_find_edges_by_source_type / _by_target_type. This
is edge-type-specific, so it finds true orphans that in_degree/out_degree miss
(e.g. a node with only a DEFINES edge but no CALLS caller). Multi-hop / nested-
WHERE EXISTS is intentionally unsupported and errors clearly.
Tests cover the dead-code (NOT EXISTS) and has-outgoing (EXISTS) cases; smoke
coverage added. ASan-clean.
Refs: Cypher read suite (EXISTS predicate)
Add an args-list (cbm_func_arg_t) to return items and parse comma-separated
arguments (var.prop or string/number literals), enabling multi-argument scalar
functions in projections:
- coalesce(a, b, ...) first non-empty value
- substring(s, start[, len]) 0-indexed substring
- replace(s, from, to) replace all occurrences
- left(s, n) / right(s, n) leading/trailing n chars
Arguments are freed in free_return_clause (ASan-clean). Unit test
cypher_func_multiarg plus smoke coverage for substring + coalesce.
Refs: Cypher read suite (tier 2b)
Add single-argument string functions to projections — size, length, trim,
ltrim, rtrim, reverse — via the generic function recognizer.
More importantly, change the engine to FAIL LOUDLY on unsupported syntax instead
of silently projecting an empty column. An unknown function call (e.g.
split(...), coalesce(...)) or list indexing/slicing in RETURN/WITH now returns a
clear "unsupported function '<name>' (supported: ...)" error rather than a
valid-looking but blank result — the same silent-empty failure mode that hid the
labels() bug. This supersedes the #373 graceful-resync behaviour (its test is
flipped to assert the error; a new test covers an unknown function in RETURN).
Decision: full Cypher Tier 3 (lists/maps/paths/comprehensions/params — a value
data-model rewrite) is intentionally NOT implemented; clear errors on
unsupported features are the higher-value, lower-risk completeness win for the
read subset agents actually use.
Refs: Cypher read suite (tiers 1-2a + unsupported-syntax hardening)
Add single-argument functions to RETURN/WITH projections, dispatched via a
generic IDENT-call recognizer so new functions no longer need a dedicated lexer
keyword:
- labels(n) -> ["<label>"]
- type(r) -> relationship type
- id(n) -> node/edge identity
- keys(n) -> JSON list of the node's non-null property keys
- properties(n) -> the node/edge properties JSON object
- toInteger/toFloat/toBoolean(x) -> numeric/boolean casts (null on non-numeric)
Previously labels()/id()/keys()/etc. fell into the unknown-function path and
silently projected empty (labels(n) returned ""); they now evaluate correctly.
Regex =~ in WHERE was already supported. Unit tests for each function plus
smoke-test coverage via the live query_graph CLI.
Refs: full Cypher read suite (tier 1 of 3)
split(f.path,'/')[0] and similar unknown function-call / index
expressions left their trailing tokens unconsumed, desyncing the parser into
a misleading default star projection (columns f.name/f.qualified_name/
f.label, one blank row per node). parse_return_item now consumes the balanced
parentheses/brackets of an unknown call so the parser stays in sync: the
requested RETURN/WITH column shape is preserved (aliases intact), sibling
aggregates like count(*) still compute, and the unsupported expression simply
projects empty instead of corrupting the whole projection. Existing scalar
forms (labels(), toInteger()) are unaffected.
count(DISTINCT x) was a parse error ("expected token type 85, got 9").
parse_aggregate_item now consumes an optional DISTINCT inside the call and
records it on the return item. Both aggregation paths track the set of unique
values per group and emit its size for COUNT(DISTINCT): the RETURN path reuses
the existing collect_lists machinery; the WITH path gains distinct_lists/
distinct_n fields. Plain COUNT and the other aggregates are unchanged.
Unit test: COUNT(DISTINCT f.label)=1 (all same label), count(f.label)=4
(non-distinct), COUNT(DISTINCT f.name)=4 (unique names). Smoke suite adds a
count(DISTINCT) query_graph check.
openCypher node-label alternation was unsupported (parse error after the
first label). parse_node now consumes `:A|B|C` into a single "A|B|C" label
string (mirroring the existing relationship-type alternation). The seed path
unions per-label results (scan_alternation_labels), and target/hop label
filters use label_alt_matches, which ORs over the alternatives. Single-label
matching is unchanged.
Unit test (Function|Module → 5 rows; Function|Class → 4) plus a query_graph
alternation check in the smoke suite. Relationship-type alternation tests
remain green.
openCypher label predicates in WHERE clauses were a parse error
("expected token type 67, got 73"). Parse `var:Label` in the WHERE leaf as
a condition with op=HAS_LABEL/value=Label, and evaluate it against the bound
node label in eval_condition (honoring NOT). Composes with AND/OR and works
in count(...) queries.
Unit test covers true/false/negated label predicates; smoke suite adds a
query_graph WHERE-label check.
The DISTINCT keyword on a WITH clause was parsed (r->distinct) but never
applied, so WITH DISTINCT silently returned duplicate rows. Add a dedup pass
in execute_with_clause that drops projected rows whose full value tuple
duplicates an earlier one (first occurrence kept), gated on wc->distinct.
No-op for the aggregation path (which already collapses per group).
Unit test (4 same-label functions collapse to 1 row; control without DISTINCT
keeps 4) plus a query_graph WITH DISTINCT check added to the smoke suite.
Confirmed working in the current query_graph evaluator and locked in:
- #240 labels() function with AS alias
- #237 DISTINCT applied before ORDER BY + LIMIT
- #252 toInteger()
- #305 count(*) with AS alias
Closes#240. Closes#237. Closes#252. Closes#305.
Adds three changes that landed together in PR #257, with a JSON-escape
hardening on the new File node properties so unusual file extensions
can't produce malformed output.
Mode A — search_graph results expose node properties_json
---------------------------------------------------------
emit_search_results in src/mcp/mcp.c now calls enrich_node_properties
for each result and parks the parsed yyjson_doc* in an out-array. The
two exit paths in handle_search_graph (success and sq_type_error) walk
that array and yyjson_doc_free each entry after yy_doc_to_str runs, so
zero-copy strings stay alive until serialization completes. Callers no
longer have to round-trip through get_code_snippet just to inspect a
node's properties.
Mode B — Cypher bare edge return surfaces full properties JSON
--------------------------------------------------------------
binding_get_virtual in src/cypher/cypher.c returns e->properties_json
(or "{}" if absent) when prop is NULL, instead of "". `RETURN r` on
an edge variable is now actually useful for inspecting timestamps,
weights, etc. without naming each property up front.
Temporal data on FILE_CHANGES_WITH edges and File nodes
-------------------------------------------------------
cbm_change_coupling_t gains last_co_change. cbm_commit_files_t and
the internal commit_t gain a timestamp field. Both git-log paths
(libgit2 and popen --pretty=format:COMMIT:%H:%ct) populate it.
cbm_compute_change_coupling carries a parallel pair_timestamps hash
table and writes the max timestamp to last_co_change. The edge
properties JSON now includes "last_co_change":<unix-epoch>.
cbm_pipeline_githistory_compute additionally aggregates per-file
change_count and last_modified into a new cbm_file_temporal_t array
exposed via result->file_temporal. cbm_pipeline_githistory_apply
upserts those values onto matching File nodes so callers can run
hotspot queries (most-changed, recently-changed) directly. Both the
sequential cbm_pipeline_pass_githistory and the parallel run_githistory
in pipeline.c free file_temporal and trigger apply when either
couplings or temporal entries are present.
The extension stored on the File node properties is now JSON-escaped
via cbm_json_escape before snprintf'ing into the props blob. The
original PR concatenated the raw extension; a hypothetical path with
a quote or backslash in the extension would have produced malformed
JSON. Practical risk was tiny (real extensions never contain those
characters) but the escape is cheap and correct.
Tests
-----
- githistory_coupling_carries_last_co_change pins last_co_change as
the max of the per-pair commit timestamps even when an unrelated
pair sits between them in the input order.
- cypher_bare_edge_return_exposes_properties_json regresses Mode B
by asserting `RETURN r` on an HTTP_CALLS edge surfaces its url_path
in the response.
- tool_search_graph_includes_node_properties regresses Mode A by
asserting search_graph's response contains the signature/return_type/
is_exported keys from a HandleRequest node's properties_json.
Existing githistory tests had their cbm_commit_files_t initializers
extended with a zero timestamp (backward-compatible — last_co_change
falls back to 0 for callers that don't supply timestamps).
Cherry-picked from #257; renamed the timestamp field types to
long long for portability across 32-/64-bit time_t platforms, used
the existing cbm_json_escape utility instead of raw concatenation
for the extension, and added explicit tests for all three changes.
Closes#257.
Co-authored-by: Adam Schulte <adamschulte148@gmail.com>
Enforce WarningsAsErrors: '*' with zero suppressions of entire check categories.
All non-FP warnings fixed at source; true false positives get per-instance NOLINT
with reasoning. Fixes include: braces around statements (1679), isolate declarations
(75), explicit casts for multilevel pointer conversions, missing includes, function
parameter documentation via NOLINT, and proper NOLINTBEGIN/NOLINTEND nesting.
Complete C implementation of the indexing pipeline (src/), parallel worker
pool, graph buffer with merge support, SQLite writer, and 1893-test suite.
Linter setup: zero warnings from clang-tidy (all checks enabled, no NOLINT
suppressions), cppcheck, and clang-format. All issues fixed at source —
proper headers for external linkage, GROW_ARRAY macro restructured to
eliminate type parameter, null-deref paths guarded, named intermediates
for suspicious-argument checks.
Martin Vogel