fix: batch 2026-08-26 — ten metric, CLI and gate fixes - #1357
Merged
Conversation
`module_doc_lines` collected the leading `//!` block and skipped blank lines and lines starting `#![`, so a *multi-line* inner attribute ended the scan: its continuation and `)]` lines are ordinary non-doc code. Every `=X.Y.Z` citation below the cut became invisible. Both directions were live — a stale citation below it passed with the gate printing OK (the false-clean the gate exists to prevent), and a file whose only citation sat below it failed the "no longer cites" branch while being correct. Drop the stop condition rather than lexing bracket depth: `//!` is only legal as a module doc, so scanning the whole file yields the rule this one-file gate wants anyway — any `=X.Y.Z` in a module doc in `node.rs` is a claim about the pin — with nothing left to truncate. `test_code_ends_the_block` becomes `test_code_does_not_end_the_block` and now pins the widened contract via a nested `mod` doc. Four cases cover the fix, all four verified failing against the old scanner: the multi-line attribute at the extractor level, the widened contract, and both gate-level directions. The stale-citation one asserts on the message, not the exit code, since a truncating scan also exits 1 there via the other branch. Fixes #1345 Claude-Session: https://claude.ai/code/session_01SYCdx8bWc2c1ZzpZYvJTRP
`apply_check_exclude` rebuilt the gate's seed list by re-reading `--paths-from`. `read_paths_from` resolves `-` to stdin, which the walk had already drained, so the second read returned an empty list: every violation anchored against the `--paths` set alone and a `[check.exclude]` / `--check-exclude` glob silently failed to match. A `git diff --name-only | bca check --paths-from -` gate therefore failed on files the project had exempted. `run_check_walk` now resolves `--paths-from` into `globals.paths` once, before the walk, and carries the materialized list out in `CheckWalk::seeds` for `apply_check_exclude`. The second read is gone rather than special-cased. `CheckExcludes::resolve` still short-circuits first, so the no-exclude run does no glob-set build and no `--check-exclude-from` read; the remediation footer still renders from the pre-materialization `GlobalOpts` clone and keeps printing the caller's `--paths-from -` spelling. Fixes #1306
`bca metrics -O json` / `bca ops -O json` wrote each per-file document from the worker that finished it, so redirecting stdout to a file gave a different byte sequence on every run at `--jobs > 1`: forty trivial Python files measured six distinct hashes over six runs, and twenty-four Rust files ten over ten. #1244 could sort the `--output <FILE>` aggregate because it holds every result before serializing; stdout has already written each document by then. Rendering is now split from emission. `GenericFormat::render` (and `render_csv`) hand the document's bytes back instead of writing them, and a reorder buffer keyed on each file's index in the walk's resolved list releases them in that order. The buffer is installed by `run_walk_tallying` for the walks `Config::streams_documents_to_stdout` identifies, rather than by each command runner, so `metrics` and `ops` cannot drift apart on it. Every dispatched path releases its slot, document or not — a skipped, unreadable, or unparseable file releases an empty marker — so the drain never stalls behind a file that emitted nothing, which is the one way this deadlocks in production and never in a test. `flush_remaining` backs that up after the walk, ahead of the runner's own failure report, so a slot a panicked worker never released costs its successors their position rather than their existence. The emitted bytes are unchanged: `-j 8` output is now byte-identical to the pre-fix `-j 1` output on pdf.js, and content-identical on DeepSpeech. Throughput over DeepSpeech (13,583 documents) is unchanged within noise; peak RSS rises from ~280 MB to ~350 MB there, the documents that complete while an earlier file is still being analyzed. Fixes #1303
`bca` printed `skipping empty file: <path>` for every file the library declined to read, but emptiness is only one of four gates: a 1-3 byte source, a multi-kilobyte binary and a UTF-16-BOM file were all announced as empty. The cause is library-side. `read_file_with_eol` collapses every skip into `Ok(None)`, so no front-end can name the real cause. Its `io::Result<Option<Vec<u8>>>` shape is public 2.x API, so the classification arrives additively: - `SkipReason` (`#[non_exhaustive]`, `Display`) with one variant per skip branch: `Empty`, `TooSmall`, `Utf16Bom`, `NotUtf8`. - `read_file_with_eol_classified`, returning `io::Result<Result<Vec<u8>, SkipReason>>`. - `read_file_with_eol` is now that call's `.map(Result::ok)`, so the two agree branch for branch by construction. The read body moves to a private `read_gated` whose two failure modes ride one `ReadStop` enum, letting every gate be spelled `?` rather than hand-wrapped in the public nested-Result shape. That lands its halstead.effort below the pre-change function's; the baseline entry is renamed, not raised. The CLI renders the reason through `warn`, which keeps owning the severity prefix. The Python and web front-ends are untouched: the web server never reads files, and the py batch path's `Ok(None)` contract (#1238) would need a widening this issue does not want. Fixes #1287
Ruby `raise`/`exit`, Perl `die`/`exit` and Tcl/iRules `error` leave a function exactly the way Python's `raise`, Go's `panic` and Lua's `error` do, but none of them has a dedicated grammar node and only the latter three had a callee-text arm. A Ruby guard clause that raised scored nexits 1 where the byte-equivalent Python scored 3. Each is matched at the seam its siblings already used, re-derived against the target grammar with `bca dump`: - Ruby: a receiver-less `call` whose `method` identifier spells the builtin. `Checker::is_call` supplies the four visible `call` aliases. - Perl: `call_expression_with_bareword`, which every call form nests exactly once, so no wrapper double-counts. - Tcl and iRules: the leading word of a generic `command`. Tcl adds the 8.6 `throw`; iRules does not, because TMOS runs a Tcl 8.4-derived interpreter with no such builtin. Receiver'd calls (`obj.raise`, `$obj->die`), package-qualified callees (`Carp::croak`) and the same words in argument position (`puts error`) stay uncounted, each with a negative test. A bare argument-less Ruby `raise` also stays uncounted: it parses as a plain identifier, indistinguishable from a variable read. `tests/parity/exit_cross_language_parity.rs` documented the gap as a language property — four fixtures using two `return`s each, which would have kept passing after this change without exercising it. They now use `return` + the abrupt-exit builtin like the other 18 languages, so the cross-language guard is live for all four; each new arm was verified by perturbation to fail its unit test and the parity test. Metric values shift for Ruby, Perl, Tcl and iRules functions using these builtins. Fixes #1270
`PhpCode::get_op_type` listed both the wrapper nodes of a PHP type annotation and a qualified name AND the leaves those wrappers contain as Halstead operands, so one occurrence billed the same bytes two to five times: `int` scored 2, `?int` 3, `Foo\Bar\Baz` 5, and `?A\B` 6 for two identifiers. #1259 fixed the `$variable` half of the same shape; this is the rest of it. Two design calls, recorded in the arm comments because the code cannot carry them: * A type's operand is its innermost concrete form. `?int` is the operand `int` — `?` is already an operator, so folding it into the operand text bills nullability across both Halstead halves and splits one type into `int` and `?int` in the vocabulary. * A qualified name's operands are its components. `Foo\Bar\Baz` is `Foo`, `Bar`, `Baz` around two `\` operators, which is exactly how PHP's own `::` and `->` already read here and how Rust's `foo::bar::baz` reads. The whole-path reading would plant a vocabulary entry per prefix. The direction differs per construct, and deliberately. `named_type`, `optional_type`, `union_type`, `intersection_type`, `disjunctive_normal_form_type`, `qualified_name`, `relative_name` and `namespace_name` drop out of the operand arm entirely, because the grammar requires each to contain the node that now carries the operand. `primitive_type` cannot: `bca dump` shows it is childless for `callable`, `iterable`, `mixed`, `void`, `false` and `true`, so dropping it would score those six types zero — grammar-dispatch §6. There the wrapper keeps the operand and the keyword child is suppressed under it, parent-scoped so the `array` heading an `array(...)` literal still counts. Metric values shift for PHP: lower Halstead N2/n2 and therefore lower volume, difficulty, effort and bugs, and a higher maintainability index. The two reproducers go 23 -> 15 and 14 -> 9 operands. The six PHP corpus snapshots are refreshed in the submodule bump, the `get_op_type` self-scan entry and the php.rs rustfmt-bail count are rebaselined for the added arms. Each guard is pinned by test-via-perturbation: re-adding the wrappers to the operand arm fails three tests, un-suppressing the keyword leaf fails four, making the keyword suppression unscoped fails only `php_primitive_type_keyword_guard_is_parent_scoped`, and dropping `PrimitiveType` instead of gating its leaf fails `php_childless_primitive_types_still_count`. Fixes #1293
The JS family, C# and Groovy classified *container* expression nodes as
Halstead operands while the walker independently reached and classified
every leaf inside them, so one member access billed three vocabulary
entries where the rest of the workspace bills two plus an operator:
var r = a.b; -> operands a, b, a.b (should be a, b)
Removed from the operand arms: `MemberExpression`/`2`/`3` in the shared
`impl_js_family_get_op_type!` body and `MemberExpression4` /
`NestedIdentifier` from the TS and TSX extras; `QualifiedName`,
`GenericName` and `AliasQualifiedName` in C#; `QualifiedName` and
`QualifiedType` in Groovy. Classification never stopped descent, so the
inner leaves were already counted and the removal deletes only the
composite entry.
Two things the containers were masking, both closed here:
* `PrivatePropertyIdentifier` (`#x`) was in no operand list, so the
composite had been the only count for `this.#x` and the declaration
`#x = 1` counted nothing at all. It joins the operand arm, or the
bare deletion would have regressed private fields to zero.
* Groovy's `QualifiedType` never fired, because the runtime emits the
alias `QualifiedType2` the arm did not name. That lesson-2 miss is
why the issue recorded Groovy as already compliant; its
`QualifiedName` half really did double-count `package com.example`.
C, C++, Java, Rust, Python, Go, Kotlin, Ruby, Lua and PHP were probed
with `bca ops` and are all already leaves-only, so this is convergence
on the existing convention rather than a new one. It is recorded in the
macro's doc comment.
Metric values shift: `n2` and `N2` fall (-29% / -18% over the pdf.js
and C# corpora), volume falls and `mi` rises; `difficulty` and
`effort` rise, because `n2` falls faster than `N2`. 331 integration
snapshots refreshed, value-only.
Fixes #1263
`variable_declaration` (`var`) and `lexical_declaration` (`let` /
`const`) had no LLOC arm in any of the four JS-family modules, so a
declarations-only file reported `lloc 0` and every real JS/TS file
under-reported LLOC by one per declaration. Java's
`LocalVariableDeclaration`, Rust's `let` and Python's assignments all
count the equivalent construct.
The two JavaScript modules also count `using_declaration`, the third
member of the grammar's `declaration` supertype that runs an
initializer; the pinned TypeScript and TSX grammars emit no such node.
Two enclosing constructs already count the row, so the declaration is
carved out under either — mirroring Java's for-header rule:
* a classic `for (let i = 0; ...)` header, whose `ForStatement` has
its own arm;
* an `ExportStatement` (`export const a = 1;`), reached by an
ancestor walk rather than a parent check because TypeScript
interposes an `ambient_declaration` for `export declare const`.
`for (const x of ...)` and `for (var k in ...)` need no carve-out: the
grammar inlines the keyword into `for_in_statement` and emits no
declaration node. `StatementBlock` stops the walk, so a declaration in
a function or loop body still counts.
Metric values move: `loc.lloc` and its aggregates rise for JS, MozJS,
TypeScript and TSX input. 351 pdf.js corpus snapshots refreshed; the
churn is LLOC-only.
Fixes #1283
The file-level unit anchors its reported span at line 1 because the unit is the file (#1195), but its `sloc` row span was still measured from the root node, which tree-sitter starts at the first token. Blank rows above that token therefore counted in neither `sloc` nor `blank`, while identical rows one line lower counted in both: `"\n\n\nfn a() {}\n"` reported `sloc 1, blank 0` over a reported span of 1..4, and the same file shifted down by a leading comment reported `sloc 4, blank 2`. Anchor the unit's `sloc` span to the span the unit reports, at space finalization. Three places held a copy of where the unit's rows start — `line_span`, `loc::shared::init`, and the synthetic-Unit seed — and #1195 updated only the first. Deriving the span from the finished `FuncSpace` leaves `line_span` the sole owner, so the synthetic seed is deleted rather than corrected. Doing it in the walker also keeps the twenty-odd per-language `Loc` impls out of it: none knows the space kind, and re-widening the trait with a unit flag is what #1067 removed for perf. This also retires the #1087 carve-out. Whitespace-only files reported `sloc 0` when newline-terminated because most grammars collapse the root to a zero-width point at end-of-input; with the span anchored, where the root starts is no longer observable in LoC and all twenty-five grammars agree. `whitespace_only_input_is_the_documented_carve_out` is renamed and now pins the absence of the split it used to enumerate. Metric values move for files whose first token is not on line 1: `sloc` and `blank` rise by the leading blank rows, their averages and maxima follow, and `mi` falls through its `ln(sloc)` term. One corpus snapshot and three CSV snapshots move; nested spaces and `exclude_tests` pruning are unaffected, both pinned. Fixes #1247
A C++ `friend` with an inline body is written inside the class braces but is a free function the class merely grants access to. `npm` never counted it as a method, yet the space tree nests its `Function` space inside the class space and `wmc` weighted every such space, so the two metrics disagreed about the same class — the #1258 divergence surviving for `friend`. Reparenting the friend's space out of the class was considered and rejected: the walk is a single-pass stack, so a space is a child of whichever space was open when the walk reached it, and `tests/parity/space_span_containment.rs` requires a child's span to sit inside its parent's. A friend's body is physically inside the class braces, so no placement outside the class can satisfy both. Instead the walk records membership at the one point that still holds the node: `Checker::is_non_member_function` (default `false`, overridden by Cpp and Mozcpp for a `function_definition` under a `friend_declaration`) is consulted in `open_func_space`, and `wmc::Stats::merge` declines a marked child's contribution to the enclosing container. Only WMC consults it; the friend keeps its own `Function` space and its own metrics, `nom` still counts it, and the file-level cyclomatic sum is unchanged. One predicate rather than a byte-less / `_with_code` pair, since the pair's silent forwarding is what makes a wrong-spelling call site read as correct (`.claude/rules/grammar-dispatch.md` section 7). The sibling sweep found the same shape in Objective-C — a `static` C function inside `@implementation` — filed as #1356 with a FIXME at the ObjC `Wmc` impl. Java/Groovy `static {}`, Kotlin `init`, TypeScript `static {}`, C# and Rust local functions, and every nested-container case were verified fine. Metric values move: `wmc.class_wmc` / `class_wmc_sum` / `total` fall by each inline friend's cyclomatic. Five DeepSpeech snapshots refreshed in the submodule. Fixes #1301 Claude-Session: https://claude.ai/code/session_01SYCdx8bWc2c1ZzpZYvJTRP
New lesson #90 (single-consumption source re-read, #1306) and sub-examples merged into #2 (#1263), #63 (#1301) and #59 (#1247). Two standing obligations go to the rule files instead: --no-fail-fast for multi-target perturbation sweeps (#1270) in testing.md and the keeper-node heuristic (#1293) in grammar-dispatch.md. Claude-Session: https://claude.ai/code/session_01SNCAwAj8i8J66tyWa9xZSR
The #1283 declaration arm carved out any declaration under a for_statement, so a brace-less for body (`for (...) var s = i;`) was mistaken for the header and never counted. The carve-out now recognises the header as the initializer field. TS/TSX also stop counting bare ambient declarations (`declare const x: T;`), which execute nothing, matching the other `declare` forms. Claude-Session: https://claude.ai/code/session_01SNCAwAj8i8J66tyWa9xZSR
Dropping MemberExpression* in #1263 removed the only operand that covered `import.meta` / `new.target`; their leaves are anonymous tokens no arm classifies, so the meta-object contributed nothing. Refreshes the two pdf.js snapshots that carry `import.meta`. Claude-Session: https://claude.ai/code/session_01SNCAwAj8i8J66tyWa9xZSR
Ruby `Kernel.raise` / `Kernel.exit` / `exit!`, Perl `CORE::die` / `CORE::exit`, and Tcl `exit` are the builtins spelled explicitly and were left out of the #1270 arms. CHANGELOG entries for #1283, #1263 and #1270 updated for the corrected behaviour. Claude-Session: https://claude.ai/code/session_01SNCAwAj8i8J66tyWa9xZSR
Tally a stdout write failure raised by the dispatch-error slot release instead of dropping it. Construct the reorder buffer through `OrderedStdout::new` (no two-phase OnceLock init), hold an Option rather than an empty-Vec sentinel, and take the stdout lock once per drain rather than per document. Materialize `--paths-from` through one `path_io` helper shared by the walk and `bca check`, and pass the seed list to `apply_check_exclude` by value. Claude-Session: https://claude.ai/code/session_01SNCAwAj8i8J66tyWa9xZSR
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1357 +/- ##
==========================================
+ Coverage 98.42% 98.44% +0.01%
==========================================
Files 278 278
Lines 73736 74392 +656
Branches 73306 73962 +656
==========================================
+ Hits 72574 73233 +659
+ Misses 756 753 -3
Partials 406 406
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Every published version of `bisync`, a transitive dependency of `gix-protocol` 0.64, was yanked on 2026-08-24, so the cargo-deny advisories check fails on any lockfile still holding it and `cargo update -p bisync` cannot satisfy `^0.3.0`. `gix-protocol` 0.65 dropped the crate, so advance `gix` 0.86 -> 0.87.1 and let the gitoxide family follow (36 lock entries, `bisync` and `bisync_macros` removed). `gix-date` 0.16 takes its reference time as a `jiff::Zoned`; the `--as-of` parser passes `gix::date::Zoned::now()` instead of `SystemTime::now()`. No behaviour or public-API change. Claude-Session: https://claude.ai/code/session_01SNCAwAj8i8J66tyWa9xZSR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Batch fix of ten open issues on one integration branch, each fix taken through simplify / optimize / review / test-audit, followed by a
/code-review max --fixpass over the whole branch whose confirmed findings landed as four follow-up commits.check-safety-doc-pinscans every//!line; a multi-line#![allow(…)]can no longer truncate the scanbca check --paths-from -materializes the seed list once, so[check.exclude]globs anchor correctly with stdin inputbca metrics/bca opsstdout emits documents in walk order via a reorder buffer — byte-identical at--jobs > 1SkipReason+read_file_with_eol_classified; the CLI names the gate that skipped a file instead of calling everything "empty"nexitscounts Rubyraise/exit, Perldie/exit, Tcl/iRuleserror(+ Tclthrow); parity guard revived#xprivate fields gain their operandvar/let/const(and JSusing) declarations count as LLOC, with for-header and export carve-outssloc/blankcount leading blank rows;line_spanis the single owner of the unit's row span (retires the #1087 carve-out)friendno longer contributes to the class WMC (Checker::is_non_member_function)Metric values move for JS/TS/TSX (
lloc, Halstead, MI), PHP (Halstead, MI), Ruby/Perl/Tcl/iRules (nexits), C++ classes with inline friends (wmc), and any file opening with blank lines (sloc/blank/MI). Each shift is called out in its CHANGELOG entry.Review follow-ups (last four
fix/refactorcommits)forcarve-out also swallowed a brace-lessforbody; keyed on theinitializerfield now. TS/TSX stop counting baredeclare const.import.meta/new.targetlost their only operand when the containers were dropped —MetaPropertyadded.nexits: RubyKernel.raise/Kernel.exit/exit!, PerlCORE::die/CORE::exit, Tclexit.--paths-frommaterialization shared between the walk andbca check.Reported, not fixed here: drain-batch write-failure under-tally (needs a
releasesignature change),inline friendmis-parse upstream in tree-sitter-cpp, the Perl fat-comma(die => 1)artifact, and the stdout-vs---outputordering difference.Follow-up issues filed by the sibling sweeps
#1351 bash, #1352 groovy, #1353 ruby, #1354 irules/tcl, #1355 perl (Halstead container/leaf double-counts); #1356 objc
@implementationC helpers weighted into class WMC (anchored byFIXME(#1356)).Validation
make pre-commit→BCA_GATE: passon the final treemake chain-auditclean for the walk-bookkeeping change (fix(metrics/loc): unit sloc/blank ignore leading blank lines, disagreeing with the anchored span #1247/fix(wmc/cpp): an inline friend's complexity is rolled into the class WMC #1301)cargo llvm-cov, lines added by this branch)big-code-analysis-outputsubmodule advanced tod2a30ddf(six snapshot-refresh commits), pushed ahead of this branchtesting.md,grammar-dispatch.md)Fixes #1345
Fixes #1306
Fixes #1303
Fixes #1287
Fixes #1270
Fixes #1293
Fixes #1263
Fixes #1283
Fixes #1247
Fixes #1301
https://claude.ai/code/session_01SNCAwAj8i8J66tyWa9xZSR