scripts/setup-windows.ps1 has 326 lines and its parse-coverage report read
"113-113,113-113,245-327" — the same line named twice, and an end line that
does not exist. Two separate faults, both in cbm_error_regions_push.
Past-EOF end line. A tree-sitter node that ends at column 0 stopped right
after the previous line's newline, so it holds no text on the row it points
at. Adding 1 to that row named a line past the end of the file whenever the
region ran to EOF. The node here is start=(244,2) end=(326,0). Clamp the end
to the row above when the end column is 0 and the node spans more than one
row.
Duplicate range. Line 113 carries two separate ERROR nodes, at columns 25-29
and 31-32, and each pushed its own range. A line range says nothing new the
second time. Drop a range that exactly repeats the one already open. The
drop runs BEFORE the cap check, so a repeat is never miscounted as a range
the cap threw away.
Only an EXACT repeat is dropped, never a range that merely overlaps. Each
range is judged separately afterwards by cbm_region_is_recovered, which asks
whether definitions starting inside that range cover it. Two ranges holding
the same numbers always get the same verdict, so dropping the repeat changes
nothing. Two different ranges do not. Merging 3-3 into 2-3 hands the wider
range's covering definition to an error that definition does not explain, and
a real parse failure then vanishes from the report.
That is not hypothetical. An earlier version of this commit merged on overlap
and broke perl_malformed_source_remains_partial_issue1838, the test added
with the Perl grammar refresh in 17b5a432. The malformed fixture produces two
ERROR nodes, at lines 2-3 and 3-3. Merged, the 2-3 range looks fully covered
by before_error and is removed, so parse_incomplete comes back false on a file
that plainly does not parse. That test now pins this boundary.
The real file reports "113-113,245-326".
Two tests, both proved RED first with the exact expected text:
coverage_repeated_error_line_reports_one_range_issue963 "2-2,2-2" != "2-2"
coverage_range_never_ends_past_the_last_line_issue963 "1-5" != "1-4"
Suites run on this change: parse_coverage 34, extraction 325, pipeline 264,
mcp 246, index_resilience 7 — all passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
The markerless stale-rendezvous recovery test reset the activation runtime
parent to NULL instead of restoring g_cli_suite_runtime_parent, so every
later cli test ran the activation probe against the developer's real
/tmp/cbm-daemon-<uid>. Restore the suite value per the contract at
tests/test_cli.c:61-65 and the sibling at :1310.
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
phase1_scan_functions() in pass_semantic_edges.c sorts the collected
Function/Method node pointers so the func order is deterministic. When
the graph has no Function or Method node at all -- a struct-only or
config-only project -- node_ptrs is never allocated and the call becomes
qsort(NULL, 0, ...). The func_count <= 0 early-out lives in phase 1b,
which runs only after phase 1 has returned, so it never guards this call.
A NULL qsort base is undefined behaviour. glibc declares the parameter
nonnull, so UBSan on the Linux leg reports "null pointer passed as
argument 1, which is declared to never be null" at the call. The macOS
SDK carries no such attribute, so neither the default sanitizer build nor
the Homebrew LLVM diag lane can flag it there. The existing
pipeline_docstring_go_class test already reaches the call on Linux; the
harness never gated on recoverable UBSan reports, so it went unnoticed.
Guard the sort on func_count > 0. The new test
pipeline_semantic_edges_no_functions indexes a Go struct-only fixture
through the full pipeline and pins the fixture to what it claims
(Struct >= 1, Function = Method = 0). On the Linux leg with
UBSAN_OPTIONS=halt_on_error=1 the pipeline suite aborts at the qsort
call before the fix and completes with it.
Distilled from #1810 with co-author credit.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Co-authored-by: Yusuf S. Demirci <ysf-samet@outlook.com>
The head of the first pattern is bound whether the query names it or not:
execute_single falls back to the synthetic name "_n0". The capacity check
counted only names, so an unnamed head plus 16 named nodes passed it and
binding_set then dropped the 16th name without a word — the same silence
this guard exists to remove.
Count that slot, and name the synthetic variable once instead of twice.
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
The complexity pass (transitive_loop_depth / recursive) walked Function and
Method nodes in temp-id order and each node's CALLS targets in adjacency
order. Under parallel extraction the temp ids come from one shared atomic
counter, so id order is worker-scheduling order and differs run to run. The
cycle guard flags whichever member of a mutual-recursion cycle the DFS enters
first, so `recursive` flipped between the members of a cycle across otherwise
identical multi-worker runs while the CALLS edge set stayed identical -- a
violation of the MT-byte-identical invariant.
Evidence: on a 66-file Python fixture of equal-sized mutual-recursion cycles
(24 pairs + 6 triangles, adjacent in the size-ordered work queue) a probe saw
3 distinct outputs in 5 runs at 4 workers. The new regression test is RED on
main 5/5 (a 4-worker run diverges from the single-threaded run on the first
cycle every time) and GREEN with this change.
Fix: order the traversal by content only -- seeds sorted by (qualified_name,
file_path, start_line), and each node's CALLS targets sorted the same way
before recursing, through one bump-stack scratch sized to the CALLS edge
count (a single upfront allocation that bails the way the existing calloc
path does). Semantics are unchanged: the DFS entry node of a cycle is the one
flagged, as before; which member that is now depends on the inputs alone,
identically for sequential and parallel indexing. SCC-based marking of every
cycle member is out of scope here.
Test: pipeline_complexity_props_independent_of_worker_order indexes the
fixture once single-threaded and six times with CBM_WORKERS=4 through the
same harness as the sequential/parallel parity test, collects every
Function's complexity props sorted by qualified_name, and asserts all
parallel runs are identical to the sequential run.
Distilled from #1925 with co-author credit.
Verification: build/c/test-runner pipeline complexity extraction (ASan/UBSan)
617 passed, 0 failed; revert-check RED 5/5 on main, GREEN with the fix;
make -f Makefile.cbm lint-ci clean; scripts/check-no-test-skips.sh OK.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Co-authored-by: Zhiyu <zhiyuzhang001@gmail.com>
Visual Basic 6 shares .frm with FORM and .cls with Apex/ObjectScript, so VB6
forms were parsed by the FORM grammar (0 defs) and class modules by the Apex
grammar (junk nodes, extract timeouts). Recognise the VB6 export header
(VERSION x.yy / VERSION 1.0 CLASS / Begin VB. / Attribute VB_Name) and report
such files as unsupported (CBM_LANG_COUNT) until a Visual Basic grammar exists:
new cbm_disambiguate_frm(), VB6 check added to cbm_disambiguate_cls() ahead of
its ObjectScript scan. Defaults on any doubt are unchanged (FORM / Apex).
Short-term step agreed in #721; the grammar follows separately.
Refs #721
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Tokarzewski <bartlomiej.tokarzewski@gmail.com>
The first cut on this branch (c1b9c451) respelled the session root and
the allowed root with forward slashes on the way into the session policy
(cbm_mcp_server_set_session_context) and at the three application.c sites
that canonicalize a root. CI's Windows shard then failed two tests:
tests/test_daemon_application.c:2256: ASSERT(sensitive_blocked)
tests/test_daemon_application.c:2380: ASSERT(sensitive_blocked)
Both set HOME to the cbm_canonical_path spelling of a directory - the
platform's native form, backslashes on Windows - and expect a session
rooted there to be refused for auto-index and for watch. The sensitive-root
and allowed-root containment walk (ws_is_ancestor_or_equal in
src/foundation/workspace.c) compares prefixes byte-exact, so a root that
now read C:/Users/... no longer matched a HOME of C:\Users\...: the
sensitive refusal was lost and $HOME was admitted. Respelling the policy
root was the wrong layer.
This commit keeps the policy roots in their canonical native spelling
(mcp.c and the three application.c sites are back to what main runs) and
folds the separator spelling at the one comparison the original defect
lives in: application_index_args_equal, where the running job's args are
compared with an explicit index_repository request's. The auto-index job
spells repo_path the way the session policy holds it; the handler spells
it with forward slashes. On Windows the two never matched, and the request
was refused as OPTIONS_CONFLICT instead of joining the job already running
for its root. The fold acts on the parsed copies only (yyjson_mut_obj_replace
with a cbm_normalize_path_sep'd string), so nothing the daemon stores
changes spelling and every other option stays exact.
Why this is correct on Windows without a local Windows run: the policy
spelling is untouched, so the containment path is byte-for-byte what main
runs there today; cbm_normalize_path_sep folds backslashes on every
platform, so the new fold is exercised by the new test wherever the suite
runs.
Tests: daemon_session_context_keeps_one_spelling_of_a_root asserted that
the policy respells a root with forward slashes. That was the wrong
contract: the policy's containment checks compare its spelling byte-exact
against HOME and the granted roots, so a respelled root is exactly what
stops matching them. It is replaced by
daemon_session_context_keeps_the_policy_spelling_of_a_root, which binds
the opposite: the policy stores the root as given. The comparison is exposed through
application_internal.h as cbm_daemon_application_index_args_equal_for_test
and bound by daemon_application_index_args_compare_repo_path_separator_equivalently:
a backslash and a forward-slash repo_path compare equal; a different path,
a sub-path, and a different mode still do not.
Refs #1726
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
CI caught the macOS run exiting 1 AFTER the watchdog assertion passed
('ok: child exited after parent death' followed by exit code 1): the
kill_hard helper resolved the Windows pid with 'ps -W', an MSYS-only
flag. Under 'set -euo pipefail' the failing ps aborts the helper, the
'[[ -n ]] && kill_hard' statement inherits that status, and the EXIT
trap turns a passing test into a red job. Linux ps would fail the same
way. Guard the pipeline (empty winpid on POSIX, where the flag does not
exist) and restore the bare '|| true' tail on every cleanup kill, which
the original script had and my refactor dropped.
Signed-off-by: 周文瑄 <zhouwx1997@126.com>
The POSIX parent-death watchdog (#407) was excluded on Windows with the
comment "job objects handle this". They do not: the KILL_ON_JOB_CLOSE
job in subprocess.c only wraps processes CBM spawns itself. An MCP stdio
server is spawned BY the client as its child, and Windows never
propagates a parent's termination to its children, so a force-killed
client (editor crash, task manager, CI timeout) leaves the server
lingering forever blocked on stdin. The orphan pins SQLite WAL read
locks, blocking checkpoints (#1083) and feeding the delete_project
permission-denied chain from #914.
Windows has no reparenting to poll for, so instead of getppid polling
the watchdog opens a SYNCHRONIZE handle to the parent at startup and
waits on it: the kernel signals a process handle exactly once on
termination, and the held handle pins the process object, so PID reuse
cannot fool the wait. The 500 ms loop timeout exists only to re-check
g_shutdown, mirroring the POSIX poll cadence. On a signaled parent the
thread takes the same deliberate _exit(0) as POSIX: after the owning
client is gone, kernel handle reclamation is the only trustworthy
release for the daemon connection, file locks and the WAL read lock.
When no trustworthy parent signal exists at startup (snapshot failed,
reserved PID, parent already exited, or the handle cannot be opened),
the client keeps running and leans on the stdin EOF path - the same
fail-open choice as the POSIX initial_ppid <= 1 bail-out - while a
watchdog thread creation failure stays fail-closed. Workers are
unchanged: they are spawned inside CBM's own kill-on-close job, which
is the containment the old comment assumed everyone had.
The parent-watchdog shell test now runs on MSYS2 instead of skipping:
an MSYS bash wrapper launches the native server over an anonymous pipe
(an MSYS FIFO is not readable by native binaries), a writer helper
holds the write end open so the exit can only come from the watchdog,
and the kill resolves the child's Windows-physical parent (the pipeline
subshell, not the wrapper script process - the watchdog watches the
Toolhelp ParentProcessId) and TerminateProcesses exactly that one,
because MSYS kill -9 does not reliably terminate the Windows process
behind an MSYS pid.
Verified against the pre-fix binary: same test, same tree, the old
build leaves the server running after the parent dies; this build exits
within one watchdog tick.
Signed-off-by: 周文瑄 <zhouwx1997@126.com>
cli_uninstall_quiesces_active_cohort_before_removing_binary_and_index moved
HOME and CBM_CACHE_DIR into a temporary directory but left PATH alone. Agent
detection reads PATH first, so a real agent binary on the developer's machine
was found even though HOME pointed at the empty fixture. Uninstall then tried
to edit that agent's config file inside the fixture, failed because the fixture
never created one, and stopped before removing the binary and the index. The
test measures exactly that removal, so it failed.
On this machine the agent is Goose, at /Users/<me>/go/bin/goose. Same tree and
same built runner, only PATH differs:
goose on PATH 291 passed, 1 failed
goose off PATH 292 passed, 0 failed
With this change the suite reports 292 passed, 0 failed while goose is still on
PATH, so the fix is the redirect and not a quieter environment.
This only stops the suite from depending on what the developer has installed.
The abort it exposed is a real defect and is reported in #1954: one agent config
that cannot be edited stops teardown for every other agent and for the
executable and indexes.
Eleven other tests that call cli_activation_save_env leave PATH open the same
way. They pass today, so I left them alone rather than change what they
exercise without evidence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
check_projection_scope models declared names in a fixed 32-entry array
and skipped the check entirely when a query declared more. Nothing
bounded how many variables a pattern could declare, so the out-of-scope
refusal added by #1922 switched itself off on a wide query: the same
undeclared name was refused at 10 declared names and quietly accepted at
35, answering a column of empty strings.
Skipping was deliberate — a wrong refusal costs the caller a working
query, which is worse than the silence. The premise was wrong. A binding
holds CYP_MAX_VARS node variables and CYP_MAX_EDGE_VARS edge variables in
plain arrays, and binding_set and binding_set_edge drop anything past
those without a word, so a query naming more cannot be answered at all.
Its extra names bind to nothing and project as blanks, which reads as
"the graph holds no such data". The choice was never refuse-or-stay-quiet;
it was refuse, or answer wrong.
So bound the input. check_pattern_var_capacity counts distinct node and
edge variables across every pattern — they share one binding, and an
OPTIONAL MATCH pattern sits in the same array — and refuses beyond what a
binding holds, naming the limit and how to get under it. Leaving the name
off a node frees its slot, because an unnamed node takes none.
Bounding the input rather than each consumer also makes three other silent
drops unreachable: binding_set past 16, binding_set_edge past 8, and the
column collection in execute_default_projection. Those would otherwise
need an error path threaded up through every caller, since both binding
setters return void.
The two `< 0` branches in check_projection_scope stay, with comments
saying they can no longer fire. They keep the guard standing if either
bound ever moves.
Fixes#1995
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
On Windows cbm_canonical_path answers in the platform's native backslash
form, and the daemon stored that spelling verbatim for the client's
session_root and allowed_root (cbm_mcp_server_set_session_context) and for
the auto-index job's repo_path (application.c canonicalizes with the bare
call at three sites). Every tool handler, however, normalizes separators
after canonicalizing. One directory therefore had two names, and the exact
comparison in application_index_args_equal never matched: an explicit
index_repository request for the session root was refused as
OPTIONS_CONFLICT instead of joining the auto-index job already running for
it.
Normalize the separator spelling of both session roots in
set_session_context, and canonicalize plus normalize at the three
application.c sites through one helper, application_canonical_root().
Separators fold on every platform, so the new test
daemon_session_context_keeps_one_spelling_of_a_root binds wherever the
suite runs; it fails on main with "C:\repos\cbm" != "C:/repos/cbm".
Distilled from #1726 with co-author credit.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Co-authored-by: 刘冲 <mail@liuchong.dev>
`uninstall` failed with rc 1 on any macOS where Hermes is installed:
Hermes is detected by its binary on PATH, not by its home, so a HOME
without `.hermes/` (every test HOME, and a fresh user's) reached the
Hermes uninstall ops with `~/.hermes/config.yaml` absent. The JSON
removers already treat a missing config as "nothing to remove"; the YAML
removers acquired the lock first, and the lock directory is created
beside the target — with the parent directory absent that failed, the
adapter recorded
error: agent_config agent=Hermes op=pre_llm_hook_uninstall ... (target: does not exist or cannot be inspected)
error: agent_config agent=Hermes op=mcp_uninstall ...
and uninstall refused to remove the executable and the indexes. This is
what made cli_uninstall_quiesces_active_cohort_before_removing_binary_and_index
fail on such a machine (7837 passed, 1 failed on a pristine main).
All four YAML removers short-circuit on a genuinely absent path and
create nothing: lstat + ENOENT on POSIX, and CreateFileW with
FILE_FLAG_OPEN_REPARSE_POINT on Windows, mirroring toml_read_file.
Opening the reparse point rather than following it keeps a dangling
symlink out of the short-circuit on both platforms, so it still reaches
the reader and its byte-identical rejection.
The regression test covers the four removers and runs on every platform:
it asserts "nothing was created" through cbm_path_info_utf8 rather than
lstat, so the Windows branch is exercised rather than compiled out.
Signed-off-by: David Gil <dgilperez@gmail.com>
cbm_refine_regions_with_pp_lines asks cbm_line_is_toplevel_macro_call about
every preprocessor-parsed line in a region. That check walked the source from
byte 0 to find the line, so the refinement cost bytes times lines — on exactly
the whole-file-ERROR C shape the refinement exists to narrow.
Build one line-offset table per file, and hand the byte span to the macro
check. The line-based form stays for the two callers that ask about a whole
region, where a single walk is cheaper than a table. A file with no table (the
allocation failed) still gets the old walk, so the answer never changes.
Measured on one C file whose file-scope brace is split by an #ifndef, padded to
5k / 10k / 20k / 40k lines. Release build, one machine, seconds:
lines main branch this commit
5000 0.98 1.14 0.98
10000 3.30 4.02 3.41
20000 11.64 14.77 12.56
40000 44.04 58.58 46.90
The branch ran 33% over main at 40k lines and now runs 6.5% over. The reported
ranges do not change: 8-8, where main reports the whole file as 5-40000.
A profile of the 40k run no longer reaches the macro check. What is left is
cbm_line_contains and cbm_span_contains_callable_def, which main carries too
and this branch does not touch. The fixture holds 40k definitions, which makes
those two cost more here than on a smaller file.
parse_coverage, extraction, pipeline, mcp and index_resilience: 964 passed,
4 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
Review follow-up on this branch.
Each parse_unusable entry carries one number, and the field was called
"lines". That reads as the length of the file, and the two are not the same
number: a grammar can end an error node past the last line, which this repo
has already met — scripts/setup-windows.ps1 has 326 lines and its range ends
at 327. A report whose whole thesis is honest reporting should not name that
number after the wrong thing.
"lines" also already means something else in this same response. Every search
result carries a "lines" field holding a definition's line span. One word, two
meanings, one document.
The field is now "range_end", at both places that emit it — add_coverage_report
reading the persisted rows, and add_parse_unusable_summary reading the per-run
errors. The comment beside each one says the number can exceed the file, so the
next reader does not have to rediscover it.
Deriving the real file length instead was the other option and is not
available here: neither cbm_file_error_t nor cbm_coverage_row_t carries it,
only the path and the range string.
One test, proved RED first — "range_end is NULL" against the old field name.
It reads the end line from the persisted coverage row rather than a constant,
so it states the property and not a measurement, and it asserts the old name is
gone rather than kept beside the new one.
index_resilience, parse_coverage and mcp: 283 passed, 4 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
Phase 5. Four test groups, each checked RED before it was kept.
- The Studio Export range join puts ONE ",+<N>" marker at the end with the
summed drop count. A marker left mid-string makes every reader stop there
and silently lose the ranges after it. Reaching the join through the
pipeline needs an export file with 256+ error regions across two <Class>
elements, so it goes through a test seam, following the pattern already in
this repo (CBM_COVERAGE_MARKER_TEST_API).
- check_index_coverage emits every range in front of a marker, never turns
the marker's digits into a range, and reports "truncated" from BOTH caps —
the producer's and its own 256 limit.
- test_index_resilience now has a ceiling beside its floor: exactly one of
the two fixture files is flagged, the clean neighbour is absent, and the
range does not cover the whole file.
- The three _Thread_local forms are pinned as measured. Only the array form
fails today; the plan's Phase 0 also listed the pointer form, and that is
wrong on the grammar shipped now.
Also fixes 13 clang-format violations the earlier commits on this branch left
in cbm.c, mcp.c and pass_definitions.c. `make -f Makefile.cbm lint-format`
would have failed CI. The changes are whitespace only — the two reflowed tool
descriptions concatenate byte-identically, so no output moved.
Full suite: 7735 passed, 28 failed, 7 skipped. The 28 are the pre-existing
cli install/uninstall failures, identical at clean HEAD.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
Two silent failures in the parse-coverage report, both made visible by the
Phase 2 range refinement that came before this.
Two caps sat in series and both returned early without saying anything:
CBM_MAX_ERROR_REGIONS = 64 internal/cbm/cbm.c
COVERAGE_RANGE_MAX = 128 src/mcp/mcp.c
Raising only the first would have moved the clip from 64 to 128, so both move
to 256. This was live behaviour, not a theoretical limit: after Phase 2 split
one whole-file range into many small ones, src/cli/cli.c and tests/test_cli.c
both reported exactly 64 ranges — the cap binding, dead-on, twice. Every
coverage figure measured before this change was a floor. With the cap at 256
the true numbers are cli.c 13.9% (not 9.8%) and test_cli.c 3.1%, and the
longest list in the repo is 85 ranges.
A raised cap is still a cap, so the report now says when it clipped:
- cbm_error_regions_t gained a `dropped` counter, and cbm_collect_error_regions
walks to the end instead of stopping at the cap, so the count is exact rather
than a lower bound. That costs little — the walk never descends into an ERROR
subtree.
- cbm_error_ranges_str appends ",+<N>" when N ranges were thrown away.
- coverage_add_ranges reads that marker and sets "truncated": true, and also
sets it when its own limit stops the loop. Before this the marker was
invisible: the parser stopped at the '+' with no error and no leftover, so a
clipped list arrived looking complete.
- objectscript_export_append_error_ranges strips markers off both operands
before joining two Studio Export parts and adds one back at the end. A marker
left mid-string would make every reader stop there and silently lose every
range after it.
"Look at lines 1 to 13047" of a 13046-line file tells a reader nothing. Those
files now carry their own kind rather than being described as partially
covered.
New `parse_unusable` field in CBMFileResult, set when one range covers 80% or
more of the file. Its customers are non-C languages: the Phase 2 refinement
that narrows a whole-file range using the preprocessed parse only runs for C,
C++ and CUDA, so a Python, Java, Ruby or TypeScript file whose root node is
ERROR still reports 1-N. Verified against real files in all four.
The kind is `parse_unusable`, not `parse_failed`. index_coverage.kind already
means one of two things — indexed-but-partial, or a skip phase saying the file
was never indexed at all — and `parse_failed` reads as the second when it is
the first. The store.c schema comment, which is the only written record of this
vocabulary, now describes all three classes and says why.
Two places would have mislabelled the new kind as "skipped", which is exactly
that confusion: coverage_status fell through to its catch-all pass, and
add_coverage_report fell into its else branch. A reader who finds a file under
"skipped" believes it is absent from the graph, when it was indexed. Both now
have explicit branches. index_status gained parse_unusable_count so a CI gate
can read it without parsing anything else, get_code_snippet says "read the
source directly" instead of naming useless ranges, and the three tool
descriptions that listed two coverage kinds now list three.
Seven added. The cap test moved from 64 to 256; a new test asserts the marker
carries a real drop count and that nothing follows it; an inverse test asserts
an under-cap file carries no marker at all. For the new kind: a Python file
whose root is ERROR is unusable, a file with a local parse failure stays
partial, a clean file is neither, and — the one that matters most — the
that last one ever flips, the Phase 2 refinement has stopped working.
Full suite: 7732 passed, 28 failed, 7 skipped. The 28 are pre-existing
agent-client install/uninstall failures in the cli suite, identical in count
and identity at clean HEAD.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
src/cli/cli.c reported an error range of 1-13047 — the whole file. The file
indexed fine; the report was wrong. Three #ifndef _WIN32 blocks split a brace
(two `if` headers, one closing brace), so the raw tree-sitter parse cannot
resync at file scope, the root node becomes ERROR, and cbm.c takes its
whole-file branch.
The pipeline already parses these files a second time after preprocessing, and
that parse is clean. The report just never consulted it.
Build one byte per original line from the preprocessed pass, then cut each raw
error range down to the runs of lines the second parse could not vouch for.
Three rules, all found by running it and all load-bearing:
- An expanded line only vouches for its original line when it HAS TEXT. The
preprocessor emits a blank line where it dropped a branch; treating that
blank as proof suppressed every C range in the suite.
- Preprocessor directive lines (with backslash continuations) never count as
missing code — the preprocessor consumes them, so the second parse can never
vouch for one. Without this every #include block reported as a miss. Known
cost: a #define the raw parse really dropped no longer shows up on its own.
- A TOP-LEVEL macro invocation line never counts as vouched-for even when the
expanded line parses clean. The macro can expand to a whole definition that
the recovery walker deliberately refuses to adopt (#949), so a clean second
parse there proves nothing. An in-body invocation is the benign #1071 case
and is left to the existing macro subtraction.
The order of the three coverage steps is now settled by where each one's
evidence lives:
recovery subtraction -> before the refinement; its evidence is a whole
definition that STARTS inside the range, so it must
be asked while the range still matches the construct
the refinement -> middle
#1071 macro rule -> after the refinement; its evidence is per-line, so a
narrow range points at the call itself
Measured on this repo: src/cli/cli.c goes from one whole-file range to 64
ranges over ~9.8% of the file, tests/test_cli.c from 48.6% to ~2.9%,
src/cli/activation_transaction.c from 38% to 7.5%. What survives is honest —
the biggest remaining ranges in cli.c are genuinely discarded #ifdef _WIN32
and #ifdef CBM_CLI_ENABLE_TEST_API blocks, absent from the graph on this
platform.
Both percentages above are floors, not measurements: cli.c and test_cli.c now
land on exactly 64 ranges, which is CBM_MAX_ERROR_REGIONS. That cap drops
regions with no signal, and a follow-up raises it and adds a truncation marker.
Five tests, all red before the change: the range narrows to the dropped
branch; lines the preprocessor explained are excluded; a range never starts or
ends on a directive; real garbage beside a split brace stays flagged; a clean
file stays unflagged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
Path.write_text is open-then-write: it creates and truncates the file
first and the content lands afterwards. The parallel harness contract
polls for `<suite>.ready` to appear and then parses its content as the
leader pid, so a reader that wins that window sees an empty file and
fails with `ValueError: invalid literal for int() with base 10: ''` --
a spurious FAIL of the harness contract with a scheduler-side cause.
The same shape applied to `<suite>.leader-exited`.
Publish the barrier files through a helper that writes a same-directory
temp file and os.replace()s it onto the destination, which is atomic on
POSIX and Windows: a poller now sees either no file or complete content.
All three barrier publications go through it.
Pin the contract structurally (no timing): the scheduler may not write a
barrier file in place, and it must rename into place instead. The pin is
RED on the previous scheduler.
Distilled from #1188 with co-author credit.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Co-authored-by: Kody <kaidi.shi.1121@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Land the four residual hardening points from #938 on the current
semantic-search design (semantic_limit/offset, the certified top-K
fetch loop):
- extract_semantic_keywords rejects a semantic_query array that holds a
non-string element instead of silently running the string subset; the
caller gets the same isError as for a bare-string semantic_query.
- run_semantic_query_core fails closed when the vector scan itself
fails ("semantic search failed", isError) instead of rendering a store
error as zero matches plus the broaden-your-keywords hint.
cbm_store_vector_search now tells the two states apart: a store with
no node_vectors table (lean index) is CBM_STORE_NOT_FOUND and keeps
the empty-page + moderate/full-index hint that main relied on by
swallowing the "no such table" prepare failure; a failing scan is
CBM_STORE_ERR.
- vs_min_cosine_score hoists the keyword norms (once per search) and
the node norm (once per node) out of the per-(node, keyword) loop.
The integer sums are exact, so every score stays bit-identical.
- vs_append_result checks its strdup results, frees the partial row on
failure and counts a row only once every copy succeeded, so the
caller's free path stays valid.
Two reproduce-first tests in the mcp suite fail on main and pass with
the change: non-string elements produce the type error, and a forced
cbm_cosine_i8 failure produces the fail-closed error for semantic-only
and combined calls while a missing node_vectors table still yields the
lean-index hint.
Refs #915
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Co-authored-by: Dustin Persek <dustin.persek@protonmail.com>
Martin Vogel