Repository navigation
fix(core-engine): four metric defects — tcl body slicing, groovy/apex args call sites, m4 arity (#2763, #2782, #2783, #2784) - #2790
Merged
Conversation
…tes (#2783) The `args` method arm's return-type group carried a trailing `?`. With the annotation and modifier runs both zero-allowed, that made the whole prefix optional and the arm degenerated to `^[ \t]*IDENT(...)` -- the exact shape of a bare call statement. Per `docs/args_rule_contract.md`, `args` matches the parameters a callable DECLARES; a call site consumes a parameter surface, it does not publish one. Same defect and same fix family as #2773 (objective-c, typescript) and #2782 (groovy). * The return type is now MANDATORY, which is what `java` and `csharp` -- the same C-family rule, same shape -- already require. Apex has no `def`: every method declaration carries an explicit return type, and a constructor carries the class name in that position (`public Foo(x)` backtracks to type=`public`, name=`Foo`). * `return` joins `new` in the return-type slot's exclusion, as csharp branch 1 already does: `return doWork(x);` is a call whose two tokens otherwise satisfy `type name(...)`. * A new branch 3 reaches the one real declaration a mandatory return type cannot -- a modifier-less constructor (`MyClass(Integer x) {`, default private) -- anchored on the body `{` that FOLLOWS the parameter list, the way java/csharp anchor their own constructor branches. It is placed LAST so the method and trigger arms keep their capture-group indices, since `_calculate_block_metrics` reads `group(lastindex)`. A `(?=[ \t\n]*\{)` lookahead was rejected as the PRIMARY anchor: it cannot reach Apex's bodiless interface and `abstract` declarations, and relaxing it to `[{;]` re-admits `foo(x);` verbatim. It is correct only as the secondary constructor branch. Measured (raw rule counts over the prism code stream, both corpora): | corpus | func_start | args before | args after | |-------------------|-----------:|------------:|-----------:| | keyword-rosetta | 13 | 18 | 13 | | language-crucible | 41 | 41 | 39 | keyword-rosetta lands on the 13 planted declarations exactly. The crucible's previous 41-vs-41 was a coincidence, not agreement: tree-sitter ground truth is 38 declarations, so the old rule was 38 real plus 3 false positives in `SOQLRecipes.cls`. Recall is 38/38 unchanged; precision goes 38/41 -> 38/39. Per-function `args_count` has zero differences across all 51 spliced functions -- the removed hits sat inside bodies, and the block search takes the signature first -- so per-function metrics are untouched. Known and deliberately not fixed here: a SOQL clause inside `[...]` of the shape `SELECT SUM(Amount) total` still reads as type=`SELECT`, name=`SUM` (1 hit in language-crucible, 0 in keyword-rosetta, unchanged by this commit). That is a different defect class -- an embedded sub-language scanned as Apex statements, which wants a `_scope_filters` entry rather than another keyword exclusion, and blanket-excluding `select` is unsafe under this rule's `re.I` per the #2671 ledger entry. Golden masters move (`SOQLRecipes.cls` file-level `args` 16->14 plus the corpus-wide X/Y/Z re-solve); blessed once for all four fixes in a later commit on this branch. Closes #2783 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sites (#2782) The `args` method arm's return-type run was `{0,3}`. At zero repetitions, with the annotation and modifier runs also zero-allowed, the arm collapsed to `^[ \t]*IDENT(...)` -- the shape of a bare call statement -- so `close(conn)`, `assertEquals(kit, kit)` and `probeBranch(argv)` all scored as declared parameter lists. Per `docs/args_rule_contract.md`, `args` matches the parameters a callable DECLARES; a call site consumes a parameter surface, it does not publish one. Same defect and same fix family as #2773 (objective-c, typescript) and #2783 (apex). The terminator lookahead was the right family but insufficient alone, so the declaration form is now two arms: * Arm 1 (bodied) keeps the fully-optional prefix -- Groovy genuinely declares `def entry(argv) {` and bare constructors `FineractPluginExtension(Project project) {` -- and anchors on the body brace via `(?=[ \t\n]*(?:throws ...)?\{)`. It is a lookahead, so `group(0)` still ends at `)`, which a strict test asserts. * Arm 2 (bodyless) reaches the casualty class a `{`-anchor cannot: Retrofit interface methods, `@Managed` model-rule interfaces, abstract Gradle property getters. Allowing a bare `;`/end-of-line here readmitted 163 matches, 60 of them Groovy's paren-less-call DSL sugar (`implementation project(':lib')`, `storePassword getReleaseKeystorePassword()`) -- the same trap typescript hit in #2773. So the construct is named instead: arm 2 makes the declaration type MANDATORY and limits it to a primitive, `def`, or an uppercase-initial type. That is the exact discriminator, since the DSL sugar's receiver is always a lowercase property name. A `{`-lookahead alone yields 890 on the crucible; arm 2 recovers 122 genuine bodyless declarations on top of that -- the measured difference between a correct fix and a merely narrower one. The parameter-list matcher is now string-aware (`[^()"']|"[^"]*"|'[^']*'|\([^()]*\)`), because once you check what FOLLOWS the list, a stray `)` inside a default value's string literal breaks the match -- `test_groovy.py`'s pathological case. This is count-neutral: the crucible match set is byte-identical with and without it. Measured (raw rule counts over the prism code stream): | corpus | func_start | args before | args after | |-----------------------------|-----------:|------------:|-----------:| | keyword-rosetta (4 files) | 13 | 19 | 13 | | language-crucible (304) | 875 | 2157 | 991 | keyword-rosetta lands on the 13 planted declarations exactly, and per file now equals the func_start column (3/3/3/4). a/f goes 2.48 -> 1.13. No corpus authoring change needed. Recall: the new crucible match set is a strict SUBSET of the old -- 0 additions, 1166 removals, verified by diffing both match sets offset-by-offset. The removals are call statements plus 7 `def (a, b) = ...` multiple-assignment destructurings, which declare locals rather than parameters and are correctly out of contract. No real declaration lost. The pattern now has 5 capture groups rather than 3 (the closure group moves 3 -> 5). This is safe: `_calculate_block_metrics` reads `group(lastindex)`, and groovy declares no index-keyed `_args_*_groups` config and no `_scope_filters` -- both confirmed empty. Pre-existing and deliberately not addressed: arm 1's zero-prefix form still admits a DSL call with a closure argument (`configure(foo) {`), the likely source of the residual a/f > 1.00. `func_start` guards that with a DSL-keyword exclusion list plus a named-argument lookahead; porting either is a separate change carrying its own risk. Golden masters move (groovy appears in both audits); blessed once for all four fixes in a later commit on this branch. Closes #2782 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ex, not the first (#2784) `m4` has no formal parameter list, so `$1`/`$@` in a macro body is the whole parameter surface. The FILE-LEVEL count of those references is intended morphology, settled in keyword-rosetta's `m4-parameters-are-use-sites` ledger entry, and this change deliberately leaves it untouched: wrapping the existing alternation in one capture group moves no match boundary, so the raw rule count is byte-for-byte identical (verified: keyword-rosetta 18 -> 18, language-crucible 70 -> 70). What moves is the PER-FUNCTION arity. m4 has no `args_search_text` bound -- that exists only for objective-c/c/cpp/dart -- so `_calculate_block_metrics` searches the whole macro body and trusts the single LEFTMOST hit. `shell` hit exactly this in #1518 and solved it with `_args_findall_max_groups`, which makes the shared counter re-scan the block and take the highest positional index via `_count_shell_positional_max`. This ports that mechanism; group 1 is the hook the helper indexes on. CORRECTION TO THE ISSUE: #2784's headline example does not reproduce. Measured old-vs-new on the per-function path: | macro body | before | after | true arity | |-----------------------------------------|-------:|------:|-----------:| | `AT_SETUP($1) AT_CHECK($1)` (the issue) | 1 | 1 | 1 | | `AS_IF([test x$1], [do_thing($3)])` | 1 | 3 | 3 | | `AC_BEFORE([$0],[AC_OTHER])` | 1 | 0 | 0 | | `do_all($@)` | 1 | 1 | 1 | A doubled `$1` already read 1, because the leftmost-hit path takes `"$1"` and stops -- so the issue's "reads arity 2" claim was wrong. The +38% it measured is purely the file-level total, which is the ledgered morphology and correctly stays. The defects the first-match-only path really does cause are the other two rows: every position after the first was silently dropped, and `$0` -- the macro's own NAME, not a parameter -- was counted as one. Both are fixed, which is the issue's actual intent (an arity should reflect declared positions), so it is closed here rather than reopened. Corpus effect, hand-verified: * keyword-rosetta: file-level 18 -> 18, per-function sum 13 -> 13, `avg_func_args` 1.0000 -> 1.0000. Nothing moves; no corpus re-authoring needed. * language-crucible: file-level 70 -> 70, per-function sum 14 -> 0, `avg_func_args` 0.3590 -> 0.0000. This is a precision gain, not a recall loss: every `$N` inside a sliced macro on that corpus is `$0` (curl-confopts 9, zz40-xc-ovr 51, xc-am-iface 3), all the `AC_BEFORE([$0],[...])` idiom inside genuinely argument-less `AC_DEFUN`s -- true arity 0, where the old 1.0 was `$0` misread as a parameter. The only real `$1`/`$2` on that corpus is in `curl/configure.ac`, which has `func_start = 0`, so no satellite gained or lost a count. No macro that actually references a positional argument went to zero. The block is safely bounded for a whole-block re-scan: `_slice_by_m4_brackets` (Mode F) cuts at the macro's own bracket-aware paren span, so no neighbouring macro's positions leak in despite the missing `args_search_text`. Golden masters move -- the crucible per-function sum flows through `avg_func_args` -> `log_avg_func_args` -> magnitude for the `m4/curl/*` entries; blessed once for all four fixes in a later commit on this branch. Closes #2784 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… audit The 46-language audit table still described apex, groovy and m4 as open violations and carried the counts they had BEFORE the three fixes on this branch. Updates each row to the measured post-fix numbers and the anchor shape that got them there. Corollary 3 is also corrected on a point the m4 work disproved. It asserted that `$1 … $1` "is one parameter referenced twice under any reading" and implied the engine read it as 2. It did not -- the per-function path takes the LEFTMOST hit, so a doubled `$1` already read 1. The real failure of a first-match-only arity is that it drops every higher position after the first (`$1 … $3` read 1, not 3) and counts `$0`, the macro's own name, as a parameter. The corollary now states the rule that actually holds: an arity is the highest position DECLARED, not the first one seen. Rows now read as fixed rather than filed: * `apex` 18 -> 13 rosetta, 41 -> 39 crucible. Records that the old 41-vs-41 was a coincidence -- 38 real declarations plus 3 false positives -- so the fix is a precision gain with recall unchanged at 38/38. * `groovy` 19 -> 13 rosetta, 2193 -> 991 crucible, a/f 2.48 -> 1.13. * `m4` file-level 18/70 unchanged (the ledgered morphology), with the correction that its `$1 ... $1` example never reproduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… not the parameter list (#2763) A tcl `proc` has TWO brace groups -- the parameter list (`proc name {a b}`) and then the body (`{ ... }`). Mode B's generic fallback starts its brace search at `start_idx`, so for tcl it always found the PARAMETER LIST and `_find_balanced_end` closed one character later. Every tcl function in every scan recorded `loc` 1, `complexity` 0 and `struct_branch` 0: 13/13 functions in keyword-rosetta, 389/389 in language-crucible. `func_start` already consumes the parameter group via its optional `(?:[ \t\n]+\{...\})?` arm, so `match.end()` is the correct body anchor. THE FIX IS GATED TO TCL, AND THAT GATING IS THE LOAD-BEARING DECISION. The blanket form -- "search from `match.end()` for every Mode B language" -- was measured first and is catastrophic. 24 languages route to Mode B; 13 have their own `elif` earlier, so 12 reach this generic fallback. Every one of them except tcl either consumes its own body `{` inside `func_start` or stops before the parameter list, so moving the anchor skips the real body and grabs the NEXT declaration's brace. Divergent matches (first `{` from `start_idx` != first `{` from `match.end()`) across both corpora: | c | cpp | scheme | groovy | powershell | apex/css/html/php/swift | |---:|---:|---:|---:|---:|---:| | 1756/1756 | 1247/1247 | 96/96 | 11 | 10 | 0 | scheme's opener is `(` rather than `{`. tcl is the only language in this fallback whose `func_start` consumes a NON-BODY brace group, so it is the only one the anchor may move for. Verified after the change: c (1756 functions) and cpp (1147) produce byte-identical per-function records, as do the other nine. On the #2758/#2760 precedent (Mode D anchoring at the declaration rather than the brace): that change carried no per-language gate and added no reusable flag, but it cuts toward gating rather than away -- it is self-gating by construction, its message justifies scope by a property "Mode D specifically" has, and it measured the broad form (601 golden-master diffs), rejected it, and shipped the narrow one (20). Precedent for measure-then-narrow. A no-brace fallback to the old anchor is retained deliberately. A tcl body need not be a brace group at all -- it is just another word, and `proc faultsim_test_proc {testrc testresult testnfail} $O(-test)` (sqlite/malloc_common.tcl:347) passes a VARIABLE as the body. Without the fallback that real declaration was DROPPED (376 -> 375). With it, every proc either gets a correct body span or exactly what it had before, so the fix is a strict superset and the function count never moves. Also fixes a second, separable defect in the same language's `_build_brace_safe_stream` arm, which carried the C-family `//[^\n]*|/\*.*?\*/` shield -- exactly perl's #1437 bug one language over. Tcl's only comment introducer is `#` (already blanked upstream by prism), but `//` appears constantly as a URL scheme separator, and shielding from it blanked the rest of the line including any `}` on it, silently unbalancing the depth counter. This was invisible before the slicer fix because a tcl body was never brace-walked at all; with it, `portfetch.tcl`'s `bzrfetch` measured loc 515 for a real 37-line proc. 14 of 376 crucible procs over-captured this way; all 14 resolve with the branch removed. Measured: | corpus | functions | loc<=1 | mean loc | branch>0 | |---------|----------:|-------:|---------:|---------:| | rosetta before / after | 13 / 13 | 13 / 0 | 1.00 / 4.62 | 0 / 2 | | crucible before / after | 389 / 389 | 389 / 5 | 1.00 / 35.90 | 1 / 272 | Recall: 0 functions lost, 0 gained on either corpus; `args_count` identical for all of them. The residual `loc<=1` rows are genuine one-line procs plus the variable-bodied one. An independent tcl-aware raw-source brace walker (more reliable here than tree-sitter-tcl, which is itself wrong on this corpus) puts functions with a verifiably correct span at 8/376 -> 370/376. Of the 2 residuals, one is an oracle artifact (nested `proc finish_test`) and one is an upstream `prism.py` defect -- `#` inside `string map {1.#INF ...}` blanked as a comment -- filed separately, out of scope here. Adds 6 regression tests to `tests/extraction/languages/test_tcl_strict.py`, each verified to FAIL on 08a08c6. Golden masters move and are NOT blessed here; the branch blesses once for all four fixes. Quantified against a clean baseline (committed GM vs unmodified-HEAD scan = 0 diffs): 585 diffs, of which 430 are topological X/Y/Z from the corpus-wide re-solve, 152 are tcl-path content fields and 3 are global roll-ups whose inputs include tcl. No off-target content change, and no function added or removed anywhere. Closes #2763 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One bless covering all four engine fixes on this branch, rather than four separate re-blesses of the same two fixtures. The full uncapped diff was regenerated and classified before blessing, because `crucible_check.py`'s own output is capped twice (50 differences in `golden_diff.py`, a 4000-char window in the wrapper) and cannot support a scoped review on its own. Method is CLAUDE.md's: rescan the corpus with the zero-dependency venv, then `deep_compare` against the committed fixture. 1374 differences, every one attributed: | class | count | |---|---:| | topological X/Y/Z coordinates | 1069 | | tcl content | 136 | | groovy content | 131 | | global roll-ups | 27 | | m4 content | 10 | | apex content | 1 | | **off-target content** | **0** | The 1069 coordinate diffs are the corpus-wide 3D layout re-solving, which happens whenever any node's mass changes and therefore ripples into languages this branch never touched; they are attributable as a class, not individually. All 305 content differences fall in the four languages the branch changes, plus 27 global roll-ups whose inputs include them (`directory_groups/tcl/*`, `directory_groups/m4/curl/m4/*`, `health/avg_documentation`, `health/avg_cognitive_load`, and the `exposures/documentation/lowest` leaderboard, whose entries are literally the tcl files whose documentation became measurable once their bodies were sliced correctly). The largest single movers are the expected consequences of #2763: `directory_groups/tcl/ sqlite/total_mass` 1970.88 -> 3819.68 and `avg_exposures/documentation` 0.0 -> 3.97, because every tcl function previously recorded `loc` 1 with no body to carry documentation or complexity. `scope_check.py --expect tcl,groovy,apex,m4` also ran; it exits non-zero because the topological re-solve touches other languages' coordinates, which is expected and is why the classification above was done directly rather than relying on its verdict. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
squid-protocol
added
the
rosetta:rebless-owed
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.
Four measured metric defects found by the 2026-09-05 pass over the keyword-rosetta bias
variance chart and the 46-language
argsaudit indocs/args_rule_contract.md. They arebatched into one PR because they share a single golden-master bless and one CI cycle;
each is a separate commit with its own evidence, so the audit trail stays per-issue.
57952db1argscounted call sites (+38%)1dc32fc8argscounted call sites (+46%)2f1cd411avg_func_argstook the first$N, not the highestb0565306f103d171argsresults back into the audit tableThe three
argsfixes#2782 and #2783 are the same defect #2773 fixed for typescript and objective-c: a
declaration arm whose return-type requirement was optional, so it collapsed to
^IDENT(...)— the shape of a bare call statement. Per the contract,argsmatches theparameters a callable declares; a call site consumes a parameter surface, it does not
publish one.
Both land on the planted count exactly on rosetta. Neither loses a real declaration:
offset-by-offset across 304 files: 0 additions, 1166 removals. The removals are call
statements plus 7
def (a, b) = …destructurings, which declare locals rather thanparameters.
38/41 → 38/39. The old 41-vs-41 was a coincidence, not agreement: 38 real declarations
plus 3 false positives in
SOQLRecipes.cls.The interesting part of both is what the anchor had to avoid. A bare
;fallback forbodyless declarations readmitted 163 matches in groovy, 60 of them paren-less DSL calls
(
implementation project(':lib')) — the same trap typescript hit in #2773. Naming theconstruct instead (mandatory type, primitive/
def/uppercase-initial) recovers 122 genuineinterface and abstract declarations that a
{-anchor alone cannot reach.#2784 corrects its own issue
The m4 fix ships, but the issue's headline example does not reproduce.
AT_SETUP($1) AT_CHECK($1)never read arity 2 — the per-function path.search()es andtakes the leftmost hit, so a doubled
$1already read 1. Measured old-vs-new:AT_SETUP($1) AT_CHECK($1)(the issue)AS_IF([test x$1], [do_thing($3)])AC_BEFORE([$0],[AC_OTHER])The real defects a first-match-only arity causes are the other two rows: every position
after the first was dropped, and
$0— the macro's own name — counted as a parameter.Both are fixed by porting shell's
_args_findall_max_groups(#1518).The file-level count is deliberately untouched (18/18 rosetta, 70/70 crucible): it is
intended morphology, settled in keyword-rosetta's
m4-parameters-are-use-sites. Wrappingthe alternation in a capture group moves no match boundary.
#2763 is gated on purpose, and the gating is the decision
A tcl
prochas two brace groups. Mode B's generic fallback searched fromstart_idx, soit always found the parameter list and closed one character later — every tcl function
in every scan recorded
loc1,complexity0,struct_branch0.The blanket fix ("anchor at
match.end()for all of Mode B") was measured first and iscatastrophic. 24 languages route to Mode B; 12 reach this generic fallback. Divergent
matches across both corpora:
c/cpp consume their own body
{insidefunc_start, somatch.end()lands on the nextfunction's brace; scheme's opener is
(. tcl is the only language in this fallback whosefunc_startconsumes a non-body brace group, so it is the only one the anchor may movefor. After the change, c (1756 functions) and cpp (1147) produce byte-identical
per-function records.
loc<=1branch>0Zero functions lost or gained. A no-brace fallback to the old anchor is retained
deliberately: a tcl body need not be a brace group (
proc faultsim_test_proc {…} $O(-test)passes a variable), and without the fallback that real declaration was dropped. The fix
is a strict superset of previous behaviour.
The commit also removes the C-family
//comment shield from tcl's_build_brace_safe_streamarm — perl's #1437 bug one language over. A URL's//blankedthe rest of the line including its
}, unbalancing the depth counter;bzrfetchmeasuredloc 515 for a real 37-line proc. Invisible before this fix, because a tcl body was never
brace-walked at all.
Adds 6 regression tests, each verified to fail on
08a08c6a.Verification
Every fix was re-measured independently by the integrator against the engine, not accepted
from its agent's report.
ruff_audit.py --cimypy_audit.py --citree_sitter_accuracy_audit.py --ci --alltri_comparison_chart.py --all --cirosetta_audit.pycrucible_check.pyLabelled
rosetta:rebless-owed; a corpus PR is owed for apex and groovy. Theirmanifests currently encode the pre-fix counts, which included the call sites this PR stops
matching. tcl and m4 pass unchanged.
argsfunc_startapex/main.clsapex/a.clsgroovy/main.groovygroovy/a.groovygroovy/c.groovyBoth languages now total 13, matching the 13 planted declarations exactly.
bis alreadycorrect in both and does not move. The corpus change is manifests-only — no authoring
change — and keyword-rosetta's ledger already tracks this as
args-call-site-counting-apex-groovy, split out in #2773's corpus pairing precisely forthese two follow-ups.
Golden masters were blessed once for all four fixes, in a single commit, after
regenerating and classifying the full uncapped diff: 1374 differences, of which 1069 are
the corpus-wide topological re-solve and 305 are content — 0 of them off-target.
Known and deliberately not fixed here, each a different defect class:
[...]of the shapeSELECT SUM(Amount) totalstill readsas
type=SELECT, name=SUM(1 crucible hit, 0 rosetta, unchanged by this PR). Wants a_scope_filtersentry; blanket-excludingselectis unsafe under this rule'sre.Iper the apex import rule: re.IGNORECASE neutralises the [A-Z] guard, so every qualified reference (foo.bar) counts as an import #2671 ledger entry.
(
configure(foo) {), the likely source of the residual a/f > 1.00.prism.pydefect blanks#insidestring map {1.#INF …}as acomment.
Closes #2763
Closes #2782
Closes #2783
Closes #2784
🤖 Generated with Claude Code