cbm_gbuf_dump_to_sqlite() emitted gbuf.dump regardless of the writer's
result, so a run that published nothing still logged node and edge counts
as if it had worked, and the failure reached the user only as the generic
"Pipeline failed. Check repo_path exists and contains source files."
The writer's temp -> final rename in publish_writer_output() returned a
bare ERR_WRITE_FAILED. #1628 taught cbm_rename_replace() to translate the
platform error into errno so callers could report why, and wired up the
stage -> final rename in pipeline.c. This is the other rename on that
path, and it is the one that runs first.
Preserve errno across the cleanup in every publish error branch. That
includes cbm_writer_open(), where the cleanup unlinks a file that was never
created: without the save it leaves ENOENT behind, so a directory that
denied the create is reported as a missing path — the #1620 case, stated
wrongly. Read the value in graph_buffer.c before the profiling macro can
clobber it, and report gbuf.dump_failed with the return code and that errno.
A sticky append failure clears errno rather than attach a reason it does not
have, and the field is omitted when there is none. No logging is added
inside internal/cbm/, which has none today.
Tests cover both halves: the dump reports a failure instead of a summary,
the writer names its truncation reason, and the publish rename's errno
survives a cleanup that itself fails.
Refs #1620, #2001.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012FxkVF773gDGNmBmsLnzN7
Signed-off-by: juan Carlos Estrada Montoya <junk151516@users.noreply.github.com>
The 15 GB kernel run completed with every phase mark under budget, but
the budget metric itself -- the charged footprint the extract gate reads
on every file pull -- reached 16.8 GB. Two defects, found with a probe at
the gate (one line per 256 MB of charge after the spill latch, with the
class table; mem.charge.probe, CBM_MEM_PHASES=1, stays in).
Worker graph buffers kept a dense id array over the global id space. The
charge climbed gradually from the 14.06 GB latch to 14.96 GB, then jumped
about 1 GB between two consecutive gate reads, and the only class that
moved in that step was gbuf_index. Worker buffers draw ids from the shared
counter, so each of the 18 kept an id -> node array spanning the whole
global id space -- 8.5M ids x 8 B, next power of two -- almost entirely
empty, and when the global id crossed 2^23 all eighteen doubled in the
same instant. Nothing asks a worker buffer by id before the merge (every
cbm_gbuf_find_by_id caller runs on the main buffer); the worker
constructor now drops the array the way it already drops the five
secondary indexes, and the main buffer answers by id after the merge.
Spill mode did not park what was already cached. The sweeps ran only on an
over-budget observation; a run that latched early (near_budget) and then
stayed under budget through extraction -- which is exactly what the first
fix produced: 14,949 MB at the extract mark -- carried the 44,797 results
cached before the latch (8 GB) into registry build and resolve, which
cannot park, and resolve ran at a charged 21.8 GB against the 15 GB
budget with nothing to stop it. The earlier runs only fitted because the
id-array burst tripped the sweeps by accident. Extraction now ends with a
final sweep whenever spill mode is on: results belong on disk before the
phases that cannot park inherit them.
Kernel, CBM_MEM_BUDGET_MB=15000, before -> after: charged high-water
16,771-16,818 MB -> 14,582 and 14,732 MB in two runs (97-98% of the
budget); the final sweep parks ~45k results and takes the charge from
14.6 to 4.3 GB before resolve, which peaks at 11.5 GB committed; wall
234 -> 224-230 s; nodes identical (8,529,729), edges inside the
run-to-run band. RSS high-water 18.7 GB is
pages already purged and not yet reclaimed: the footprint never exceeds
7.3 GB at a mark.
peak_charged_mb (7b77fd48) is what made this measurable: the kernel at
the default budget reads charged 22.3 GB against 24.6 GB while ps reports
29 GB RSS, and the 15 GB run read 16.8 GB where every mark was under.
Tests: gbuf_worker_buffer_keeps_no_by_id_array (graph_buffer: one node at
id 2M in a worker buffer must not grow the index class by even 1 MB where
the dense array costs 16 MB; the worker answers by QN, the main buffer by
id after the merge) -- RED on the dense array, GREEN without it.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
strtod answers 0.0 for text it cannot read, and 0.0 is a real confidence
in both places that call it. So a property blob carrying a malformed
value, such as "confidence":null, came back as a recorded confidence of
zero. Two things went wrong with that.
In src/graph_buffer/graph_buffer.c, edge_props_confidence answers
CBM_EDGE_CONF_ABSENT (-1) when an edge carries no confidence, so any real
confidence outranks it. A malformed value answered 0.0, which beats -1 in
the merge comparison, so the malformed blob displaced a clean stored one.
The function's own comment already promised that "absent/unparseable
reads as -1". Only the absent half was true.
In src/mcp/mcp.c, bfs_edge_evidence_for_hop sets the confidence to -1 and
the emitter publishes any value of 0 or more as a recorded number. A
malformed value printed as 0.00, which reads as "the resolver was certain
this is wrong" rather than "nobody wrote a number here".
Both sites now pass an end pointer to strtod and keep the absent sentinel
when the pointer never moved, which is the shape src/store/store.c:402
already uses.
Two tests come with the change, and both were seen failing before the fix
and passing after:
gbuf_edge_props_unreadable_confidence_does_not_displace_absent
tool_trace_path_unreadable_confidence_reports_not_recorded
Red: 258 passed, 2 failed. Green: 260 passed, 0 failed.
The full suite reports 7633 passed, 2 failed. Both failures are in
tests/test_cli.c (lines 1749 and 6725) and reproduce on a clean tree
without this change. They depend on the coding agents installed on the
machine, not on this change. make -f Makefile.cbm lint-ci passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
confidence, strategy and via are not part of the edge dedup key
(source:target:type), and the same logical edge is legitimately minted by more
than one strategy carrying different values — pass_calls emits CALLS edges from
both LSP resolution and registry-textual matching. cbm_gbuf_insert_edge
replaced the stored blob unconditionally, and per-worker edge buffers merge in
worker-slot order, so which strategy's attributes survived was a function of
thread scheduling: the edge set stayed stable while its attributes flickered
run to run.
Replace the overwrite with a total order over the two candidates, which makes
the result commutative and associative and therefore independent of arrival
order:
1. higher "confidence" wins, which also enforces the intended precedence
that an LSP-resolved call outranks a textual match;
2. on equal confidence, the lexicographically greater blob wins, giving a
deterministic choice between otherwise equal candidates.
An empty or absent incoming blob still never displaces stored properties.
Adds three tests: order independence for the same pair of blobs inserted both
ways, the higher-confidence winner regardless of arrival, and the existing
empty-blob guard. The first two fail before this change.
Signed-off-by: Andrew Studnicky <a.j.studnicky@gmail.com>
Distills PR #770 (graph-buffer dedup key) and completes the edge-uniqueness
contract it left partial: the contract lives in THREE places, and changing
only the buffer ships databases that violate their own UNIQUE constraint
(PRAGMA integrity_check: non-unique entry in sqlite_autoindex_edges_1).
A single 'import { A, B } from ./lib' produced ONE IMPORTS edge: the graph
buffer dedups edges on (source_id, target_id, type) and merge-replaces
properties on collision, so the second symbol silently overwrote the first.
pass_calls.c, pass_usages.c, pass_semantic.c and pass_lsp_cross.c parse one
local_name per IMPORTS edge for cross-file resolution, so the dropped
symbol's calls failed to resolve too — not just "who imports X" queries.
Changes, kept in sync across all three sites:
- graph_buffer.c (from #770): make_edge_key() folds local_name into the
dedup key for IMPORTS edges only; all three key call-sites updated.
Hardened beyond #770: EDGE_KEY_BUF bumped to 256 and oversized
local_names are re-keyed with an FNV-1a hash of the full name instead
of being silently truncated (two long names sharing a prefix must not
collide back into one edge).
- store.c: edges gains local_name_gen, a VIRTUAL generated column
(IMPORTS -> coalesce(json_extract(properties,'$.local_name'),''),
else '' — NOT NULL because NULLs never conflict in a UNIQUE index);
uniqueness widened to UNIQUE(source_id, target_id, type,
local_name_gen) and the insert upsert's conflict target matches.
init_schema probes pre-#768 DBs (no local_name_gen) and fails the
open: SQLite cannot ALTER a table constraint in place, and an
unopenable DB already takes the existing repair path — full index
deletes + rebuilds, artifact import refuses and falls back to a
reindex. Read-only query opens skip init_schema and keep working.
- sqlite_writer.c/.h: dump DDL matches the widened schema; the hand-built
sqlite_autoindex_edges_1 comparator and entry builder include the
local_name column; CBMDumpEdge carries local_name extracted via real
JSON parsing (yyjson) so index entries match json_extract's unescaped
values exactly, per the idx_edges_url_path precedent.
- artifact.h: CBM_ARTIFACT_SCHEMA_VERSION 1 -> 2 so old binaries refuse
artifacts carrying the widened schema (their 3-column conflict target
can no longer prepare against it).
Tests (reproduce-first, all red on the unfixed code):
- gbuf tier: multi-symbol import -> 2 IMPORTS edges; long-local_name
truncation guard.
- store tier: distinct local_name coexists as 2 rows, same local_name
still upserts, non-IMPORTS dedup unchanged.
- writer tier: dumped DB passes integrity_check with 2 sibling imports
and exposes matching local_name_gen values.
- end-to-end: TS fixture through the real pipeline -> 2 queryable
IMPORTS edges AND integrity_check ok; with only the buffer half of
the fix this test still fails on integrity_check, proving the schema
half is required.
Closes#768.
Co-authored-by: Alexandros Pappas <11921291+apappas1129@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@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.
