fix(metrics/loc): six LOC fixes from the 2026-09-10 residue - #1446
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1446 +/- ##
========================================
Coverage 97.97% 97.98%
========================================
Files 359 359
Lines 92992 93425 +433
Branches 92561 92994 +433
========================================
+ Hits 91113 91542 +429
- Misses 1211 1216 +5
+ Partials 668 667 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| /// `.claude/rules/grammar-dispatch.md` §7 is about. | ||
| fn rust_attributed_item<'a>(node: &Node<'a>, ancestors: Ancestors<'a, '_>) -> Option<Node<'a>> { | ||
| match ancestors.parent(node) { | ||
| Some(parent) if parent.child_count() <= forward_attribute_scan_budget(ancestors) => parent |
There was a problem hiding this comment.
Location: big-code-analysis-ast/src/checker.rs:807
HIGH -- Combined depth and attribute width makes this scan quadratic
should_skip_subtree calls this once for every AttributeItem. In this branch each call starts again at the parent's first child, skips to the current attribute, and then scans to the decorated item. Because forward_attribute_scan_budget permits width proportional to depth (3 * depth), a parent at depth D with 3D consecutive attributes takes this branch for every attribute and performs quadratic sibling work.
I reproduced this in release mode with valid Rust containing D nested functions followed by 3D #[cfg(test)] attributes on one inner function. bca metrics --no-config --jobs 1 --language rust --metrics nom --exclude-tests took 0.04s, 0.18s, 1.00s, and 4.62s for D=250/500/1000/2000, while D=2000 without --exclude-tests took 0.02s. The existing depth-only and width-only probes cannot expose this diagonal shape. Avoid restarting from child zero for each attribute, and add a scaling probe where depth and one attribute run grow together.
There was a problem hiding this comment.
Confirmed and fixed in 9c21c7e3. Reproduced worse than reported here — 0.08 / 0.34 / 1.21 / 5.18 s for D=250/500/1000/2000 against 0.05 s with the flag off.
Both causes turned out to be one: every attribute in a run decorates the same item, so all L answers are equal, and each ask re-derived the run twice (skip_while from child 0, then again inside rust_item_is_test_only). Rather than making the scan cheaper, a verdict now reports how far it reaches — should_skip_subtree returns SubtreeSkip instead of bool, the attribute arm sets the reach to the decorated item's start byte, and the walk reuses the last reaching verdict. Pre-order visits start_byte non-decreasing, so the test is O(1) and a passed reach cannot be re-entered. One ask per run.
Same input now: 0.00 / 0.01 / 0.02 / 0.07 s.
forward_attribute_scan_budget is unchanged — measured, the forward pass is still the cheaper of the two readings on the diagonal, and the budget was never the cause. rust_item_is_test_only is reused verbatim, so no third reading of the run was introduced (§7).
New probe nom/deep-attribute-run grows depth and one run together. Watched failing: 2.01, over bound with the reach disabled; 1.06 with it. Its reading is nom.total() of the retained wrappers rather than anything the prune produces, so a later change to what is pruned cannot zero it.
One you did not raise and I am not fixing here: the other diagonal — D nested fns with 3D separate one-attribute items, runs of length 1 so nothing to reuse — is still quadratic (3.07 s at D=1000). About half predates #1431; it is the item path's own rust_outer_attr_marks_test restarting per item. Filed separately.
| HeredocRedirect => { | ||
| check_comment_ends_on_code_line(stats, start); | ||
| stats.ploc.lines.insert(start); | ||
| stats.ploc.lines.insert_range( |
There was a problem hiding this comment.
Location: src/metrics/loc/bash.rs:161
MEDIUM -- This turns valid continuation-only source into a blank row
The new lower bound skips a valid Bash continuation row when that row contains only \. This input passes bash -n:
cat <<EOT | \
\
grep x
body
EOTAt this head, bca metrics --metrics sloc,ploc,blank reports sloc=5, ploc=4, blank=1; the previous wrapper-wide range credited all five source rows. The comment above acknowledges the regression and defers the general case to #1445, but this PR newly makes an already-correct heredoc result wrong for valid input. Preserve continuation-only prefix rows here, or land the general continuation-row fix before narrowing this range, and add this valid fixture with ploc=5, blank=0.
There was a problem hiding this comment.
You are right, and it changes the disposition: the narrowing is reverted in 9c21c7e3.
Neither remedy you offered was available. Telling a \-only row from an empty one needs the source bytes, and Loc::compute is not given them, so "preserve continuation-only rows here" cannot be written; and landing the general fix first means widening the Loc trait across twenty languages.
So this falls to the rule the sibling #1430 note in this same PR states: fix only when it costs nothing on valid input. It costs something here — what the narrowing fixed needs bash -n to reject the file, what it broke runs — so #1443 becomes a documented gap like #1430 rather than a fix. The arm now records why the narrowing looks right and is still wrong, so a second attempt fails review rather than shipping.
The fixture is inverted to your suggested values: bash_heredoc_continuation_row_is_code pins ploc 5, blank 0 on exactly your input, plus the content-carrying continuation as the bracket. General gap tracked as #1445.
| // backslash runs into, 3 is `int x;`. | ||
| const DANGLING_CONTINUATION: &[u8] = b"#define A 1 \\\n 2 \\\n\nint x;\n"; | ||
|
|
||
| for lang in [ |
There was a problem hiding this comment.
Location: src/metrics/loc.rs:11779
MEDIUM -- This regression test invokes disabled language parsers
The table unconditionally includes C, C++, Mozcpp, and Objective-C. Under a feature subset that enables none of them, the test is still compiled and panics on its first row instead of being absent. Reproduced with:
cargo test -p big-code-analysis --no-default-features --features go --lib a_dangling_macro_continuation_does_not_credit_the_row_below
... LanguageDisabled(C)
Gate the function on the union of the four features, gate each row on its own feature, and retain a non-vacuity assertion as required by the repository's feature-table test rule.
There was a problem hiding this comment.
Fixed in 9c21c7e3. Gated on #[cfg(any(feature = "c", feature = "cpp", feature = "mozcpp", feature = "objc"))], the hardcoded array replaced by a per-row #[cfg] table, and assert_fixtures_present kept for non-vacuity — both halves, per the rule.
Verified with your reproducer: under --features go the test is now absent (0 passed; 0 failed), not failing and not vacuously passing; under --features cpp it runs on the single enabled row.
Also fixed the sibling a_continued_macro_body_counts_every_row_it_spans, which has the identical shape two lines away. That one is pre-existing rather than introduced here, but it is the same edit and leaving it would have made the file inconsistent with itself.
| attribute, looking ahead over its run to the item it decorates and | ||
| pruning only when that item is one the prune removes anyway; the | ||
| attribute on a `use`, a `struct`, or any other kind `--exclude-tests` | ||
| keeps is untouched. **Metric drift:** `loc.ploc`, `loc.sloc` and the |
There was a problem hiding this comment.
Location: CHANGELOG.md:168
MEDIUM -- The public drift note omits Tokens and Halstead changes
The new attribute arm continues before compute_per_node, so it removes the attribute subtree from every selected metric, not only LOC. Rust attribute tokens are Halstead operators/operands, so tokens and the entire halstead block also fall; MI can move through Halstead volume as well as SLOC.
This is material drift rather than a theoretical detail: an independent base/head measurement over 463 Rust files found tokens and halstead.length changed in 206 files under --exclude-tests, with no change when the option is off. Please name tokens and halstead.* here and note both MI inputs, rather than limiting the documented drift to LOC rows.
There was a problem hiding this comment.
Fixed in 9c21c7e3, and you are right that it is material rather than theoretical — I had scoped the note to LOC because that is what the issue was about, not because I had checked.
Measured on one fixture to confirm the mechanism before rewriting: tokens 33→6, halstead.length 22→4, halstead.volume 83.8→8.0, mi.original 115.8→148.5.
The entry now says the prune arm continues before the per-node computes so the attribute subtree leaves every selected metric, names tokens and the whole halstead block, states that MI moves through both of its inputs rather than SLOC alone, and carries your 206-of-463 figure.
Code Review: fix(metrics/loc): six LOC fixes from the 2026-09-10 residueVerdict: CHANGES RECOMMENDED GitHub does not permit the PR author to submit a formal request-changes review on their own PR, so the findings are posted as comments.
Findings
Review passes executed
Files reviewed: 23 |
| // where `Mi::inputs_are_empty` takes over, and the issue raised that | ||
| // as a hazard: it is not one — a file with nothing in it scores | ||
| // zero, deliberately and for every formula. | ||
| #[test] |
There was a problem hiding this comment.
Location: src/spaces_tests.rs:659
MEDIUM -- The Rust-only module runs when the Rust feature is disabled
This module constructs RustParser unconditionally. The new tests therefore fail under supported feature subsets without Rust; for example:
cargo test -p big-code-analysis --no-default-features --features go --lib an_all_test_file_measures_zero_rows
... LanguageDisabled(Rust)
Add #[cfg(feature = "rust")] to the module so these tests are absent rather than spuriously failing when their parser is not compiled. The new Rust-specific tests around rust_loc_pruned in src/metrics/loc.rs need the same gate.
There was a problem hiding this comment.
Fixed in 9c21c7e3. #[cfg(feature = "rust")] on the exclude_tests_rust module, and the same gate on rust_loc_pruned and its three callers in src/metrics/loc.rs — helper and callers together, so no unused-function warning appears under a subset.
Verified: all six named tests absent under --features go, with zero warnings in that build.
One deliberate omission — the older rust_loc helper and its 28 callers are also ungated. That is pre-existing (#1051 era) and part of the #1285 pile, so I left it rather than widening this PR; say the word if you would rather it came along.
Five findings, one HIGH and four MEDIUM. **The attribute lookahead was quadratic** (checker.rs). #1431 made should_skip_subtree answer for every AttributeItem, and each ask re-read the whole run -- once in rust_attributed_item's skip_while from child 0, once again inside rust_item_is_test_only. D nested fns with 3*D attributes on one item took 0.08 / 0.34 / 1.21 / 5.18 s for D = 250/500/1000/2000, against 0.05 s with --exclude-tests off. Both readings were the same waste: every attribute in a run decorates the same item, so all L answers are equal. A verdict now reports how far it reaches -- should_skip_subtree returns SubtreeSkip rather than bool, Rust's attribute arm sets the reach to the decorated item's start byte, and the walk reuses the last reaching verdict. Pre-order visits start_byte non-decreasing, so the test is O(1) and a passed reach cannot be re-entered. One ask per run. The same input now runs in 0.00 / 0.01 / 0.02 / 0.07 s. forward_attribute_scan_budget is unchanged: on the diagonal the forward pass is still the cheaper of the two readings, and the budget was never what made this quadratic. rust_item_is_test_only is still reused verbatim, so no third reading of the run was added (grammar-dispatch section 7). The new nom/deep-attribute-run probe grows depth and one run together -- the axis the depth-only and width-only probes cannot see. It reads nom.total() of the *retained* wrappers, not anything the prune produces, so a later change to what is pruned cannot zero it. Watched failing: 2.01 and over bound with the reach disabled, 1.06 with it. **A valid Bash continuation row became blank** (bash.rs). #1443 narrowed the heredoc wrapper's range to the literal's own rows. What that fixed needs bash -n to reject the file; what it broke runs: a continuation row holding only `\` carries no leaf for the leaf-gated catch-all, so the blanket range was the only thing covering it, and `cat <<EOT | \` + `\` + ` grep x` went from ploc 5 to ploc 4. Reverted, by the rule the #1430 note states -- fix only when it costs nothing on valid input. The reasoning stays at the arm so a second attempt does not ship, and the general gap is #1445. **The drift note understated the change** (CHANGELOG). The prune arm continues before the per-node computes, so the attribute subtree leaves every selected metric. tokens and the whole halstead block fall too, and mi moves through volume as well as ln(sloc): 206 of 463 Rust files move tokens and halstead.length under --exclude-tests. **Two feature-gate gaps** (loc.rs, spaces_tests.rs). The C-family macro tests hardcoded four languages with no gate and panicked LanguageDisabled under a subset enabling none; exclude_tests_rust built a RustParser unconditionally. Both now absent rather than failing under --features go, with the non-vacuity assertions the rule requires. Fixed the pre-existing sibling test alongside.
Review findings addressed —
|
| # | Severity | Disposition |
|---|---|---|
| 1 | HIGH | Fixed — 5.18 s → 0.07 s at D=2000, new diagonal probe watched failing |
| 2 | MEDIUM | Reverted the #1443 narrowing rather than patching it |
| 3 | MEDIUM | Gated, absent under --features go; pre-existing sibling fixed too |
| 4 | MEDIUM | Gated, six tests absent, zero warnings |
| 5 | MEDIUM | Drift note now names tokens, halstead.* and both MI inputs |
Two are worth a second look because they changed the shape of the PR rather than just patching it.
Finding 1 turned out to have one cause, not two: every attribute in a run decorates the same item, so all L answers are equal and the run was being re-derived L times. The fix makes a verdict report how far it reaches — should_skip_subtree returns SubtreeSkip instead of bool — so the walk asks once per run. That is a cross-crate signature change on a trait explicitly outside the stability contract, with one override, one call site and one direct test; flagging it rather than burying it. forward_attribute_scan_budget is untouched, having been measured not to be the cause.
Finding 2 reverses #1443 outright. Neither remedy offered was writable — distinguishing a \-only row from an empty one needs source bytes Loc::compute does not receive — and by the rule the #1430 note states in this same PR (fix only when it costs nothing on valid input), the narrowing should never have landed. It is now a documented gap like #1430.
Filed from this round: #1447, the adjacent diagonal that remains quadratic — D nested fns with 3D separate one-attribute items, where runs of length 1 leave nothing to reuse. About half predates #1431; it is #1100's budget design on the item path. Neither the fix nor the new probe covers it, and the issue says so.
Also carried over: #1445 (a bare \ continuation row reads as blank, general to Bash) and #1444 (upstream: two heredocs on one command).
9c21c7e to
cff4ec2
Compare
Three lines in `src/metrics/loc/shared.rs` could be perturbed without failing any of the ~3,330 lib tests: `add_multiline_string_ploc`'s parent gate in either direction, and `add_string_interior_ploc`'s `end_line().saturating_sub(1)` upper bound. None of the three is observable end-to-end. `LineSet::insert` and `check_comment_ends_on_code_line` are idempotent, and every language routing a literal through the helper ends its match with an unconditional catch-all that has already credited the literal's start row, so crediting it again changes nothing and skipping it drops nothing the catch-all does not restore. The bound differs from the raw end row only for a node whose end column is 0. The three new tests therefore call the helpers directly, seeding a row the assertions can be measured against rather than starting from a default-empty `Stats`. The bound test uses a Bash `heredoc_body` — the one literal here that ends at column 0 — at helper level rather than through `bca metrics`, so a change to what `bash.rs` routes (#1412) cannot silently disarm it. Verified by perturbation: always-credit fails only the parent-started test, never-credit fails only the parent-did-not-start test, flipping the comparison fails both, and swapping the bound for `end_row()` fails only the interior test. A control perturbation elsewhere in the same file failed 276 tests, so the harness discriminates. Fixes #1414
`heredoc_body`'s span starts at the first body row that has text and collapses to zero width when the body has none, so a leading empty row sat inside no node at all and `blank = sloc - ploc - cloc` claimed it: `cat <<EOT\n\nEOT` reported `blank 1`. #1260 routed the body, which fixed only the interior of a body that has text -- the shape all of its fixtures had. The arm now routes `heredoc_redirect`, the node present for every spelling, as #1396 did for PHP. Both `heredoc_body` symbols are dropped rather than kept alongside it: tree-sitter-bash 0.25.1's node-types.json lists `heredoc_body` as a child of `heredoc_redirect` and of nothing else, and the wrapper runs from `<<` to the end of `heredoc_end`, so it is a strict superset in every spelling. Dumped and confirmed for `<<`, `<<-`, quoted, empty, unterminated, and heredocs in a function, subshell, pipeline, command substitution and `&&` list. Routing the wrapper also credits the interior rows of a multi-row `heredoc_content`, which the leaf-gated catch-all reached only the first row of. `BashCode::is_string` deliberately keeps omitting `HeredocRedirect` -- a redirection is an operator, not a string literal. The divergence from the parity cross-walk is recorded at the arm. Measured over all 438 shell files in the corpora, with the fix perturbed out and back in: exactly one file moves, openfst-1.6.7's ltmain.sh, by `ploc 7551 -> 7554` and `blank 795 -> 792` -- a clean swap with `sloc` and `cloc` unchanged. The two cross-language sweeps that are the home for this property used `line1\nline2\nline3`, which passes whether or not a textless interior row is credited; that is why they saw neither #1396 nor #1412. Their interior row is now empty (C excepted -- its multi-row literal is a backslash continuation, so it cannot express one), each gained the PHP rows it never had, quoted and heredoc alike, and both gained the `#[cfg(any(feature = ...))]` union that makes them absent rather than panicking under a feature subset (#1285). Fixes #1412
The `PreprocArg` arm in the C, C++, `mozcpp` and Objective-C `Loc` impls credited rows up to tree-sitter's raw `end_row()`. That over-reads by one whenever a node ends at column 0 — the shape a macro body whose last continuation is a dangling backslash produces, since the node finishes at the *start* of the row below rather than inside it. `Node::end_line` exists to encode that rule and every other multi-row PLOC path already used it. All four arms now call `add_string_interior_ploc`, of which they were an open-coded copy with the wrong bound. That fixes the bound, drops a redundant second `kind_id().into()` (the match scrutinee is bound instead), and replaces a per-row loop with one bitmap-span write. The defect is live, not latent: DeepSpeech's `left_test.cc` has a `TEXT_TEST` macro of exactly this shape and moves from `ploc 344, blank 35` to `ploc 343, blank 36`. The issue's "not currently observable" sweep measured the `ploc <= sloc` contract, which the over-credit never violated, rather than the snapshot values. Python's `String` arm was the last remaining open-coded copy of the pattern and is folded onto `add_multiline_string_ploc`. The bound change is unreachable there — a Python `string` node always closes on a `string_end` quote, so its end column is never 0, and an unterminated literal parses to a bare `string_start` under an `ERROR` rather than to a `string` node — so the fold is for reuse. C# has a `PreprocArg` and no arm for it. `tree-sitter-c-sharp` accepts a backslash continuation the C# specification does not, so the multi-row shape is grammar-reachable and language-invalid; left alone with a `FIXME` and filed as #1430 rather than guessed at. Fixes #1423
`Sloc` accumulated each `exclude_tests`-pruned subtree's whole row
span as a scalar count, on the assumption its own doc comment stated:
that rustfmt gives every Rust item dedicated rows, so a pruned span
shares no physical line with a retained sibling and the counts add
without an interval merge. Anything hand-written, generated, minified
or concatenated breaks it. `fn a() {} #[cfg(test)] mod t { … }`
reported `sloc 0, ploc 1` — one row of code in a zero-row file — and
fed that zero to MI's `ln(sloc)` term, the #1051 shape.
`excluded_lines: usize` becomes `excluded_rows: LineSet`, the single
source of truth: no cached count that can fall out of step with it,
which is the trade `ploc`/`cloc` already made in #1109. A new
`LineSet::subtract` clears `other`'s bits in place — destructive
rather than a `difference_len` counter, because `Sloc::merge` folds
the residual upward and the identity of the surviving rows has to
outlive the call. `Stats::settle_excluded_rows` runs it against
`ploc.lines` and both `cloc` sets once per space at finalization,
between the #1398 clamp and `compute_minmax`, so the subtrahend is
settled before anything reads `sloc()`. `Sloc::merge` unions instead
of adding, which also stops two pruned siblings on one row costing
that row twice.
Not the cheaper clamp of the count to `span - |retained rows|`. That
under-reports whenever the space has a blank row to absorb the
phantom subtraction, which is not a corner: on
`fn a() {} #[cfg(test)] mod t {}` / blank / `fn b() {}` the truth is
`sloc 3, blank 1`, the bug gives `sloc 2, blank 0`, and the clamp
gives `min(1, 3 - 2) = 1`, so `sloc 2, blank 0` — no improvement at
all. It buys `ploc <= sloc` by moving the error into `blank`.
Measured against the built binary, and pinned by a regression test
that fails under the clamp.
With `P`, `O`, `C` and the pruned set all clamped inside the span,
the settled residual is disjoint from `P ∪ O ∪ C`, so
`sloc() >= max(ploc(), cloc())` follows with no dependence on grammar
shape or walk order. `clamp_line_sets_to_span` gains a fourth
`retain_range` to establish that premise, and the tightened pair of
`debug_assert!`s moves to the new pass — running on every space of
every walk, which is how the fix is evidenced across all twenty
languages rather than only on the named fixtures. The clamp keeps its
own `<= span` pair, which localises a clamp failure to the clamp.
Ordinary rustfmt-shaped input is byte-identical: the seven
`exclude_tests_rust` LOC fixtures and
`exclude_tests_pruning_composes_with_the_unit_anchor` are unchanged,
because their excluded and retained rows are already disjoint. Two
observable changes to `Sloc`, both on private state reached through
derives: `Debug` prints the rows, and `PartialEq` strengthens.
`big-code-analysis-bench` gains `loc/wide-cfg-test-mod`, the only
scaling probe on which the *number* of pruned nodes grows with the
size parameter — the two `nom/*-attributed-fn` probes use `#[inline]`
precisely so the walk does not prune, so the prune arm's own row
bookkeeping was unpriced. It measures 1.21 against a 1.50 bound.
The remaining over-count is separate and filed as #1431: a
`#[cfg(test)]` attribute is an `AttributeItem` sibling of the item,
so it is walked normally and its row stays in PLOC. A 100%-test file
still reports `ploc 1`.
Fixes #1417
#1412 replaced Bash's HeredocBody / HeredocBody2 arms with the heredoc_redirect wrapper, on the grounds that node-types.json lists heredoc_body as a child of heredoc_redirect and of nothing else. That describes the well-formed grammar. Under error recovery tree-sitter-bash emits an orphan body with no wrapper at all: a single-line compound carrying a heredoc parses to an {ERROR} root whose direct child is the heredoc_body. The leaf-gated catch-all then credits only its start row and the terminator row falls to blank -- #1412's own symptom, on three valid and executable spellings: f() { cat <<EOT; } if true; then cat <<EOT; fi for i in 1; do cat <<EOT; done Each reported blank 1 rather than blank 0. Restore both body kinds alongside the wrapper, per grammar-dispatch section 6: narrow with a gate, never by deletion. Keeping both costs nothing where the wrapper exists, since the body's row range is contained in it and every insert is idempotent. The corpora carry no heredoc of this shape and the snapshot submodule has no shell files at all, which is why a green gate did not see it. Two claims that shipped with #1412 were also false and are corrected here. Routing the wrapper does not fix a multi-row heredoc_content -- those rows were already credited by the HeredocBody2 arm, and generate-pc.sh measures identically on both sides. And metrics.md has said since #722 that pruning leaves unit-level loc.sloc at the full file extent; it drops, measured 7 to 3. Cover the two settle_excluded_rows subtractions #1417 left unguarded: deleting both comment subtractions failed 0 of 3,388 lib tests, while deleting the ploc one failed 3. Every #1417 fixture has cloc 0. Both new tests were verified by perturbation, each the only failure for its own perturbation.
`subtract` and `intersection_len` are the two `LineSet` methods with no `reserve` to lean on for their index arithmetic, so each open-coded the same clip to the overlap of the two word arrays, and the module header's `arithmetic_side_effects` carve-out had to state the bounds argument for both at once. Extract `overlapping_words`, which carries that argument once: the range is empty rather than inverted when the sets are disjoint, and every index in it is in bounds for both operands by construction. Behaviour is identical.
A perturbation sweep over this branch found one production line no
test could fail on and one fixture with no decay anchor.
`settle_excluded_rows`'s third subtraction, over
`code_comment_line_starts`, failed 0 of 3,390 lib tests when deleted,
where dropping either of the other two fails 3 and 1. It is redundant
by construction: the two `add_cloc_lines` writers test
`ploc.lines.contains(start)` first, and every per-language call site
of `check_comment_ends_on_code_line` inserts that same row into
`ploc.lines` on the next statement. A `debug_assert_eq!` now pins that
premise on every space of every walk, so the line is defence with a
live guard rather than unreachable weight. Verified to fire by
dropping the precondition from `add_cloc_lines`.
`a_blank_row_does_not_absorb_a_phantom_exclusion`'s `shared_row` keeps
all four numbers when the pruned `mod t {}` is trimmed out of it, and
no axis can change that: an excluded item sharing its only row with
retained code is what "metrically invisible" means. Recorded in the
doc comment rather than papered over or overstated, per
`.claude/rules/testing.md`. Its `own_rows` sibling is anchored in the
ordinary way -- trimming that `mod` fails `sloc 3`.
An outer attribute is an `AttributeItem` sibling of the item it marks, not a child, so `--exclude-tests` pruning the item never reached it and Rust's `Loc` catch-all credited its start row to PLOC. A file that was nothing but test code reported `sloc 1, ploc 1`, and every `#[cfg(test)] mod tests` cost its file one phantom row per attribute row. `should_skip_subtree` now also answers for an `AttributeItem`, looking ahead over its run to the item it decorates and pruning only when that item is one the prune removes anyway — so the attribute on a `use` or a `struct` is untouched. Answering before the row is recorded, rather than retracting it afterwards, is what keeps #1417's shared-row case correct: a row the attribute happens to share with retained code is still inserted by that code. The lookahead dispatches on the same depth-scaled budget as the backward run reading, for #1100's reason with the directions swapped. Complexity class is unchanged on all 28 scaling probes (`nom/wide-attributed-fn` 1.02 to 1.04, `nom/nested-attributed-fn` 1.00 to 1.02, `loc/wide-cfg-test-mod` 1.13 to 1.09). The constant is not free: a generated 20_000-item `#[inline] fn f() {}` file goes 245 ms to 305 ms in release, while this repository's own 90 kloc of Rust does not move off 59 ms. `loc/wide-cfg-test-mod` gains a retained `fn p()` per item — its reading was the surviving attribute row, which is now zero. Metric drift: `loc.ploc`, `loc.sloc` and the MI values derived from them fall for `--exclude-tests` runs over Rust with test items. The default (`--exclude-tests` off) is byte-identical, and the six `loc.ploc` baseline entries that moved are refreshed here. Fixes #1431
heredoc_redirect spans the command-line prefix as well as the literal, and the grammar lets that prefix cross rows -- a pipeline is one of its children. Crediting the wrapper's whole interior, as the sibling arm does for a literal that is its own span, therefore billed every prefix row as code: a blank row read as code, and a comment-only row was reclassified from comment to code-and-comment. Start the range at the first body row instead, derived from the last row any non-body child occupies. For the single-row prefix every runnable heredoc has, that is start + 1 -- the same arithmetic as before -- so no valid input moves. The body node's own start row cannot serve here: per #1412 its span begins at the first body row carrying text and collapses to zero width for an empty body, so it cannot say where the body begins. The multi-row form is a bash syntax error. bash begins the body on the line after the one carrying <<, so `cat <<EOT |` + newline + `grep x` makes grep x the body's first line and the terminator is never found; bash -n rejects the bare, backslash-continued, comment-row, blank-row and && spellings alike, while tree-sitter parses each as a pipeline inside the wrapper. This is a tree-sitter-bash divergence, recorded at the arm rather than worked around. It is fixed anyway because bca is asked to measure malformed trees, which is #1398's argument. The floor on the computed row fails no test -- measured, 0 of 3,397 -- so it carries a debug_assert on the premise that makes it inert rather than a comment asserting the shape cannot arise. That runs on every heredoc of every walk. Fixes #1443
C# has a preproc_arg and its four C-family siblings all route theirs through add_string_interior_ploc, so the absence here reads as an oversight. It is a decision, and #1423 left it as a FIXME rather than settling it. Settle it as a gap, documented at the catch-all. The C# specification terminates a pp-directive at the new-line and defines no line continuation, so `#region Big \` ends there and the row below is a syntax error rather than an argument row. tree-sitter-c-sharp is over-permissive relative to the language, accepting a backslash continuation the way the C grammar legitimately does. A preproc_arg in runnable C# is therefore always single-row, and on a single row the arm would no-op -- so adding it could only change the number reported for source that does not compile, where a line count has no correct answer. The note records the measured reproducer, the parse it produces, why this is the opposite call to #1443 (there the range's *meaning* was wrong and correcting it was free on valid input; here the meaning is already right and only invalid input separates the two behaviours), and what would reopen the question. No test: pinning the reported numbers for invalid C# would encode the gap as a contract and invert into a bug-lock the moment the grammar tightens. The decision rests on a specification fact, not on behaviour. Comment-only; metric output is unchanged. Fixes #1430
#1443's rationale said every multi-row heredoc prefix is a bash syntax error, "verified with bash -n on five spellings". False. The five spellings all happened to carry a blank or comment row; a continuation that carries content is valid Bash and produces a genuinely multi-row heredoc_redirect: cat <<EOT | \ cat <<EOT && \ grep x echo y Both pass bash -n. Issue #1443's own body had this right and the resolution comment overturned it with the worse measurement -- the mechanism lesson 94 describes, committed in the same branch that describes it. The true claim is narrower and is what the bound actually needs: a blank or comment-only prefix row is unreachable, because bash begins the body on the line after the << and a backslash splices the rows rather than leaving one empty. All four such spellings are rejected; re-measured before writing this. One valid shape does move, which "no valid input changes" denied: a continuation row holding only a backslash has no leaf to credit it and now reads blank. That is a general Bash gap -- `echo a | \` + `\` + ` grep b` shows it with no heredoc in sight -- that the arm's old blanket range masked in one position. Filed as #1445 and referenced from the arm; #1443 stands, since the new reading is at least consistent with the same shape everywhere else. Corrected at every site that repeated the claim: bash.rs, the loc.rs test doc, csharp.rs's cross-reference, CHANGELOG.md and grammar-dispatch.md. Also fixes a stale "0 of 3,397" to the measured 3,394. Lessons: trims 84 to the 75-line ceiling, spells debug_assert_eq! correctly, and reduces 94's second sub-example to a clause -- it was lesson 74's mechanism in a new language, which the overlap rule says is not a second sub-example. Two garbled sentences in testing.md rewritten. Comment-only; metric output is unchanged.
Five findings, one HIGH and four MEDIUM. **The attribute lookahead was quadratic** (checker.rs). #1431 made should_skip_subtree answer for every AttributeItem, and each ask re-read the whole run -- once in rust_attributed_item's skip_while from child 0, once again inside rust_item_is_test_only. D nested fns with 3*D attributes on one item took 0.08 / 0.34 / 1.21 / 5.18 s for D = 250/500/1000/2000, against 0.05 s with --exclude-tests off. Both readings were the same waste: every attribute in a run decorates the same item, so all L answers are equal. A verdict now reports how far it reaches -- should_skip_subtree returns SubtreeSkip rather than bool, Rust's attribute arm sets the reach to the decorated item's start byte, and the walk reuses the last reaching verdict. Pre-order visits start_byte non-decreasing, so the test is O(1) and a passed reach cannot be re-entered. One ask per run. The same input now runs in 0.00 / 0.01 / 0.02 / 0.07 s. forward_attribute_scan_budget is unchanged: on the diagonal the forward pass is still the cheaper of the two readings, and the budget was never what made this quadratic. rust_item_is_test_only is still reused verbatim, so no third reading of the run was added (grammar-dispatch section 7). The new nom/deep-attribute-run probe grows depth and one run together -- the axis the depth-only and width-only probes cannot see. It reads nom.total() of the *retained* wrappers, not anything the prune produces, so a later change to what is pruned cannot zero it. Watched failing: 2.01 and over bound with the reach disabled, 1.06 with it. **A valid Bash continuation row became blank** (bash.rs). #1443 narrowed the heredoc wrapper's range to the literal's own rows. What that fixed needs bash -n to reject the file; what it broke runs: a continuation row holding only `\` carries no leaf for the leaf-gated catch-all, so the blanket range was the only thing covering it, and `cat <<EOT | \` + `\` + ` grep x` went from ploc 5 to ploc 4. Reverted, by the rule the #1430 note states -- fix only when it costs nothing on valid input. The reasoning stays at the arm so a second attempt does not ship, and the general gap is #1445. **The drift note understated the change** (CHANGELOG). The prune arm continues before the per-node computes, so the attribute subtree leaves every selected metric. tokens and the whole halstead block fall too, and mi moves through volume as well as ln(sloc): 206 of 463 Rust files move tokens and halstead.length under --exclude-tests. **Two feature-gate gaps** (loc.rs, spaces_tests.rs). The C-family macro tests hardcoded four languages with no gate and panicked LanguageDisabled under a subset enabling none; exclude_tests_rust built a RustParser unconditionally. Both now absent rather than failing under --features go, with the non-vacuity assertions the rule requires. Fixed the pre-existing sibling test alongside.
`src/spaces/compute.rs::metrics_inner` recorded a `halstead.effort` of 119147.75 against a live measurement of 116715.61. Nothing on this branch touches `src/spaces/`; the entry went stale on `main`, in `d4479cdf` (PR #1446's review-findings commit), which changed `compute.rs` after the last baseline refresh. It went unnoticed because the value *fell*. The baseline filter suppresses a violation only while the live measurement stays at or below the recorded value, so a decrease never reddens the gate and nothing forces a refresh — the recorded number simply stops describing the tree, and the gate runs looser than intended until someone regenerates for an unrelated reason. Here that reason was #1407 refreshing a Kotlin entry, since the write target regenerates the file wholesale. Recorded separately from that fix so the metric commits carry only their own baseline movement. The staleness mechanism is filed as #1465.
1 participant
Six LOC issues, all residue of the 2026-09-10 batch (#1396 PHP heredoc, #1398
the line-set clamp) plus the older #1260. Three of the six were filed by that
work.
add_multiline_string_ploc's parent gate (both directions) andadd_string_interior_ploc'send_line()-1bound. A fourth unguarded branch turned up and is covered tooLocroutes theheredoc_redirectwrapper, so a heredoc body that is empty at the top or throughout stops reading as blankPreprocArgarms — and Python's open-coded copy, which the issue did not name — go throughadd_string_interior_ploc, bounding rows byNode::end_lineSloc::excluded_linesbecomes a row set, settled against retained PLOC/CLOC at finalization, so--exclude-testssubtracts only rows the prune removes--exclude-testsprunes the#[cfg(test)]attribute with the item it marksPreprocArgarm, with the reasoning recorded where its absence would otherwise read as an oversightNotable
--exclude-testscould reportslocbelowploc(#1417) — a contractviolation, not a rounding artifact.
Sloccounted each pruned subtree's wholerow span, including rows retained code shares, on an assumption its own doc
comment spelled out and rustfmt-shaped input never violated. The fix carries the
pruned rows as a
LineSetand subtracts only what the prune genuinely removed;the
debug_assert!at the clamp is tightened from the raw span tosloc(),which makes the invariant a theorem rather than an observation and checks it on
every space of every walk.
Three issue bodies were wrong in ways that changed the work. #1412's
predicted snapshot churn does not exist (the submodule holds no shell files),
while #1423's — which its issue called unobservable — does, and moved
left_test.cc. #1417's expected numbers ignored that the#[cfg(test)]attribute is a sibling of the pruned item.
Two later commits are self-corrections.
e10ed92brestores theheredoc_bodyarms thatcd439806deleted:node-types.jsondescribes thewell-formed grammar only, and under error recovery tree-sitter-bash emits an
orphan body with no wrapper, which reintroduced #1412's own symptom on three
valid inputs.
22e99725corrects a reachability claim behind #1443 that wasmeasured too broadly, and files the valid-input gap that correction exposed
(#1445).
Follow-ups filed
#1444 (upstream:
tree-sitter-bashcannot parse two heredocs on one command),#1445 (a bare
\continuation row reads as blank — general to Bash, previouslymasked inside heredocs).
Verification
make pre-commitgreen on every commit. Patch coverage 97.61% againstproject 96.93%, measured per-line over the added lines; the seven uncovered
are defensive branches and
debug_assert!message arguments.make bench-scalingall 28 probes within bound,make chain-auditclean. Thesnapshot submodule is bumped to
98c48d2cfor the one DeepSpeech file #1423moves.
Also adds lesson #94, merges a sub-example into #84, and adds four
entries to
.claude/rules/{testing,grammar-dispatch}.md.