Skip to content

fix: two causes of run-to-run graph variation and three silent content losses - #2974

Merged
DeusData merged 5 commits into
mainfrom
fix/graph-variation-and-content-loss
Nov 13, 2026
Merged

DeusData merged 5 commits into
mainfrom
fix/graph-variation-and-content-loss

Conversation

@DeusData

Copy link
Copy Markdown
Owner

What does this PR do?

Five independent fixes, one commit each. Two remove causes of an index giving a different graph from one run to the
next; three stop content from being lost without a word. All five were found by scoring main against compiler-grade
references and by re-indexing the same checkouts repeatedly.

Commit Defect on main Effect
fix(pkgmap) With two manifests declaring one package name, the winner depended on which worker parsed which cli/cli: 3 of 70 runs had 690 IMPORTS instead of 3,869 and 12,392 CALLS instead of 19,898
fix(grammar) The vendored jsonnet scanner kept its state in file-scope variables shared by all worker parsers grafana/loki's 68 jsonnet files: 30 different results in 30 runs, 21-37 files reported partial
fix(graph-buffer) cbm_gbuf_insert_edge reported success for an edge the primary array could not take The edge was missing from the written graph and a retry appeared to succeed
fix(pipeline) One byte that is not UTF-8 in source text made the strict JSON writer refuse the document The repository lost every surface row and unresolved-call capture switched off
fix(cs-lsp) The C# using-directive walk dropped children once its 256-entry stack was full Later using static and alias directives were never collected

The fixes

Package map (src/pipeline/pass_pkgmap.c). cli/cli has its own go.mod and a test fixture's copy with the same
module path. cbm_pkgmap_build kept the first entry per name in worker order, so the fixture won whenever worker 0
happened to parse it, and the module root pointed into the fixture directory. Both outcomes reported
status: indexed. The candidates are now ordered by package name, depth below the repository root and path before
the first-wins insertion; the sequential builders go through the same function. With the fix, 100 of 100 runs on the
same checkout give the normal graph.

Jsonnet scanner (internal/cbm/vendored/grammars/jsonnet/scanner.c). tree-sitter restores a scanner's state
right before each scan and saves it right after; with shared variables, another thread's write in that window
changed the scan in flight. Its deserialize also ignored the zero-length call that starts a parse, so a file ending
inside an unterminated quote left that state to every later parse in the process. The state now lives in the
per-parser payload and is reset on every deserialize; lexing logic and the 3-byte serialized state are unchanged.
With the fix, the 68 files give one result in 30 of 30 runs with none partial, and the whole repository one result
in 5 of 5.

Graph buffer (src/graph_buffer/graph_buffer.c). The function compares the array length before and after the
push; on a failed push it frees the edge and returns 0 before any index sees it.

Non-UTF-8 source (src/foundation/json_writer.h, two call sites). cbm_json_write_text writes a valid document
exactly as before. Only a refused document is written again with the flag pair the layout writer already uses, so
each undecodable byte becomes an escaped U+FFFD. That flag pair comes from the fix for #2907 (ba22c965a); thanks to
@mvanhorn, whose approach this reuses for the pipeline. On a repository with only valid text (hugo), 2,178 surface
rows and 757 unresolved rows are byte-identical before and after.

C# using directives (internal/cbm/lsp/cs_lsp.c). The stack keeps 256 entries inline and doubles on the file's
arena when it is full. No new cap and no budget.

This PR changes a vendored grammar and the CI lint gate

  • Local patch on vendored code. internal/cbm/vendored/grammars/MANIFEST.md records the jsonnet patch next to
    the properties scanner's (the same defect class), THIRD_PARTY.md notes it must be re-applied on re-vendor, and
    scripts/vendored-checksums.txt is repinned for the two changed files.
  • New lint in lint-ci. lint-vendored-scanners (scripts/lint-vendored-scanners.py) rejects mutable
    file-scope state in grammars/*/scanner.c and the headers beside them (132 files). It reports exactly the three
    jsonnet variables on main's scanner and nothing on this tree. Four read-only cases in haskell and purescript are
    whitelisted with their reasons; a whitelist entry that no longer matches anything fails the gate.
  • Test-only allocation fault. cbm_mem_test_fail_next_realloc is compiled only with CBM_MEM_TEST_API, which
    the test and TSan flag sets now define.

Tests

Every fix has a test that fails on main for the right reason, passes with the fix, and fails again with the
production change reverted:

Suite Test On main
edge_imports ei_pkgmap_duplicate_package_name_prefers_root_manifest_in_any_worker_order, ..._at_equal_depth_is_settled_by_path 86 passed, 2 failed
parse_coverage jsonnet_scanner_state_is_per_parser, jsonnet_scanner_state_not_carried_between_parses 71 passed, 2 failed
graph_buffer gbuf_edge_append_oom_is_not_success 63 passed, 1 failed, 3 of 3 runs
pipeline_semantic_manifest_repro, str_util pipeline_non_utf8_source_keeps_surfaces_and_unresolved_capture, json_write_text_keeps_valid_documents_and_repairs_refused_ones surfaces=0 complete=0, 3 of 3 runs
cs_lsp cslsp_using_after_many_namespace_blocks 64 passed, 1 failed

The two variation tests need no threads and no timing: the package-map test hands the same manifests to the merge in
both worker orders, and the scanner test runs a second parser from inside the first parser's input callback.

Local, on the combined tree under ASan and UBSan: edge_imports, parse_coverage, graph_buffer, mem,
dyn_array, mem_events, str_util, pipeline_semantic_manifest_repro, cs_lsp and grammar_regression, 584
passed, 0 failed, no sanitizer report; the pipeline and doc_mentions suites passed on the UTF-8 fix alone (740
passed with str_util and pipeline_semantic_manifest_repro). clang-format and cppcheck on the changed
production files, the memory-core, NOLINT and vendored-scanner lints, and the vendored checksum comparison (1,096
entries). The full local ladder was not run.

Not in this PR

Related defects found while doing this are recorded for follow-up, among them: cbm_gbuf_upsert_node and the merge
path ignore a failed growth the same way; Go interface methods and TypeScript keyof members are cut at 63; and
the graph also differs between one worker and many, which makes it depend on the machine's core count.

Checklist

  • Every commit is signed off (git commit -s)
  • Tests pass locally (make -f Makefile.cbm test): only the affected suites were run locally, PR CI runs the rest
  • Lint passes on the changed files, including the new lint
  • New behavior is covered by a test (reproduce-first for bug fixes)

…package

Two manifests can declare the same package name: cli/cli has its own go.mod and
a test fixture's copy with the same module path. cbm_pkgmap_build kept the
first entry in worker order, and which worker parses which manifest is thread
scheduling, so about one index run in thirty mapped the module to the fixture
directory. IMPORTS edges fell from 3,869 to 690 and CALLS from 19,898 to
12,392, with status "indexed" and nothing logged.

The candidates are now ordered by package name, depth below the repository
root and path before the first-wins insertion, so the winner is a property of
the repository. The sequential builders go through the same function.

Two tests hand the same manifests to the merge in both worker orders.

Signed-off-by: Martin Vogel <[email protected]>
Indexing go/grafana__loki produced a different graph from one run to the
next: each index reported one or two jsonnet files as parse_partial, a
different set every time, and indexing only its 68 jsonnet files reported
21-37 of them, with real definitions and their CALLS/DEFINES/IMPORTS/USAGE
edges missing. Parsed one at a time, every one of those files is clean,
and CBM_WORKERS=1 gave the same graph on every run.

The vendored jsonnet external scanner kept its state (inside a string,
and which quote) in three file-scope globals, so every indexing worker's
parser shared one copy. tree-sitter restores the scanner state right
before each scan and saves it right after; another thread's write in that
window changed the scan in flight, and a quoted string ran past its
closing quote. Its deserialize also ignored the zero-length call that
starts a parse, so a file ending inside an unterminated quote left that
state to every later parse in the process, and no later ||| text block
could be lexed.

The state now lives in the payload from ..._external_scanner_create(),
and deserialize resets it before reading. Lexing logic and the 3-byte
serialized state are unchanged. MANIFEST.md records the local patch next
to the properties scanner's, THIRD_PARTY.md notes it must be re-applied
on re-vendor, and scripts/vendored-checksums.txt is repinned.

Two single-threaded tests in parse_coverage pin it: a second parser run
from inside parser A's input callback, at the byte after A's opening
quote, must not break A's string; and a parse that ends inside a string
must not stop the next one from lexing a ||| block. Both fail on the
unpatched scanner. With the patch, 30 indexes of the 68 files and 5 of
the whole repo each give one result, with no jsonnet file partial.

lint-vendored-scanners (now part of lint-ci) rejects mutable file-scope
state in vendored scanners and the headers next to them. properties and
jsonnet both had it; haskell's and purescript's read-only cases are
whitelisted with their reasons.

Signed-off-by: Martin Vogel <[email protected]>
…grow

cbm_gbuf_insert_edge ignored a failed cbm_da_push: it registered the edge in
the dedup and secondary indexes and returned a positive id while the primary
array, which the dump and free read, did not hold it. A retry then hit the
dedup entry and appeared to succeed, so the edge was lost and leaked. Compare
the count around the append; on failure free the edge and return 0 before any
index registration.

Pinned by gbuf_edge_append_oom_is_not_success, which fails the edge array's
first growth through a new test-only, one-shot, class-filtered realloc failure
hook (cbm_mem_test_fail_next_realloc, compiled only with CBM_MEM_TEST_API in
the test and TSan flag sets).

Signed-off-by: Martin Vogel <[email protected]>
… source

yyjson's strict writer refuses a whole document over one byte that is not
UTF-8, and source text can be in any encoding. Two pipeline documents carry
it: the per-file LSP surface and the unresolved-call detail. One such byte
made surface_row_build fail the whole job, so the repository lost every
surface row and each later incremental run declined to a full rebuild
(no_surface_rows). It also latched unresolved-call capture off for the rest
of the run.

Add cbm_json_write_text to foundation/json_writer.h. A valid document is
written exactly as before, so surface shas and details do not change. Only a
refused document is written again with the flags the layout writer already
uses (#2907): each undecodable byte becomes an escaped U+FFFD.

Pinned by pipeline_non_utf8_source_keeps_surfaces_and_unresolved_capture (a
latin-1 byte in a string-named TS method and in a Python __all__ entry) and
json_write_text_keeps_valid_documents_and_repairs_refused_ones.

Signed-off-by: Martin Vogel <[email protected]>
The using-directive walk kept its pending nodes in a fixed 256-entry
stack and silently skipped every child it had no room for. Children are
pushed in source order, so a wide file lost its last subtrees: with 300
namespace blocks before it, the namespace holding `using static
System.Math;` was never walked and its unqualified `Sqrt` call bound to
nothing. The stack now starts inline and doubles on the file's arena.

Pinned by cslsp_using_after_many_namespace_blocks, which asserts the
C.X -> Math.Sqrt call behind 300 namespace blocks.

Signed-off-by: Martin Vogel <[email protected]>
@DeusData
DeusData merged commit 09bd6bb into main Nov 13, 2026
43 checks passed
DeusData added a commit to htarnacki/codebase-memory-mcp that referenced this pull request Nov 13, 2026
Brings in main up to 09bd6bb (DeusData#2967 FreeBSD peer pid, DeusData#2974 graph
variation and content loss fixes).

Resolutions:
- scripts/memory-core-baseline.txt (textual conflict): both sides
  lowered counts in the same hunk. pass_parallel.c keeps this branch's
  29 (arena QNs); pass_pkgmap.c takes main's 80 (DeusData#2974 dropped one raw
  free in cbm_pkgmap_build; this branch's pass_pkgmap.c additions add
  no raw site). Both values re-derived by scanning the merged tree with
  scripts/lint-memory-core.py; the lint passes.
- src/pipeline/pass_pkgmap.c (auto-merged): main's deterministic
  manifest order in cbm_pkgmap_build and this branch's Scala package
  index sit in separate functions; the merged file is main plus this
  branch's change unchanged (same per-file patch-id).
- tests/test_edge_imports.c, tests/test_pipeline.c (auto-merged): both
  sides' new tests are kept and registered; main's new tests call no
  function this branch re-signed. Same per-file patch-id as this
  branch's change.

Every other file equals main.

Signed-off-by: Martin Vogel <[email protected]>
DeusData added a commit that referenced this pull request Nov 13, 2026
Brings in #2974 (incl. the jsonnet scanner patch) and #2970. One conflict:
scripts/vendored-checksums.txt, where #2974 and the vba vendoring both
rewrite the MANIFEST.md line. Resolved by taking main's file and
regenerating it with scripts/security-vendored.sh --update; against main it
differs only in the MANIFEST.md line and the five vba lines, and the
integrity verify passes.

THIRD_PARTY.md, MANIFEST.md and tests/test_parse_coverage.c merged cleanly;
each equals main plus this branch's own change (identical patch-ids), and
test_parse_coverage.c has 76 TEST and 76 RUN_TEST entries (71 at the merge
base, +2 from main, +3 from this branch). The MANIFEST summary (164
grammars) and ABI line (9/78/77) match a recount from the tree.

Signed-off-by: Martin Vogel <[email protected]>
DeusData added a commit to ankitsusheel/codebase-memory-mcp that referenced this pull request Nov 14, 2026
Brings the branch up to main d68f491 (DeusData#2967, DeusData#2968, DeusData#2970, DeusData#2974).

src/pipeline/registry.c: main (DeusData#2968) gives registry_resolve_scoped the
calling definition's test verdict (`caller_test`), adds
cbm_registry_resolve_ref for references (USAGE, READS, WRITES) and
routes calls through a static resolve_call_lang. This branch wraps the
lookups with the VB.NET case-insensitive retry. Resolved by keeping
both: vb_resolve_folded takes `caller_test` and passes it to its own
lookup, and the retry hooks cbm_registry_resolve_lang (false),
cbm_registry_resolve_ref and resolve_call_lang (the caller's verdict),
exactly as main's first lookup in each. Without the hook in
cbm_registry_resolve_ref, VB.NET references would have stopped folding.

internal/cbm/vendored/grammars/MANIFEST.md: main added the sql row next
to the vbnet row this branch edits; both rows kept.

scripts/vendored-checksums.txt: regenerated with
scripts/security-vendored.sh --update; only the MANIFEST.md line
differs from main.

Adapted with the merge: the VB.NET registry test passes `false` as the
new caller_test argument of cbm_registry_resolve_call_scoped (a
production caller).

Signed-off-by: Martin Vogel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant