fix(detector): stop Mode A's _slice_by_labels from truncating/discarding real bodies - #1959
Conversation
…ing real bodies (#1949) Two independent bugs in _slice_by_labels (assembly/cobol/fortran/abap/ agc_assembly's shared label-based slicing path): 1. assembly_returns truncated a function's body at ANY occurrence of a return-like keyword, including inside a doc-comment ("// @return..."), inside a larger hyphenated identifier (WS-EXIT-RETRY-LOOP), and on legitimate mid-body control flow that isn't actually a terminator (Fortran's loop-break EXIT, ABAP's guard-clause RETURN/EXIT, COBOL's block-closing END-IF, AGC's interrupt-handler-opening RELINT). Fixed by requiring a match to be a standalone statement (start of line, modulo leading whitespace), removing END-IF/END-PERFORM/RELINT outright as never-real terminators, and per-language-excluding EXIT/RETURN where corpus evidence shows they're legitimately non-terminating. 2. Any sliced body that collapsed to one non-blank line was silently discarded -- but single-instruction "trampoline" labels are completely normal in this language family (agc_assembly's SOPTION1-SOPTON10, zero- instruction labels in assembly bootloaders), so Mode A's "greedy to the next label" boundary is already a correct function boundary without needing a minimum line count. Confirmed via agc_assembly's real pipeline DB: 52 real, ctags-corroborated labels (11.6% of raw matches) were being dropped before this fix; after, every file in the corpus has zero gap between struct_func_start and function_count. Spot-checked against all 5 real corpus files cited in the issue across assembly/cobol/fortran/abap -- all now show zero gap. Regenerated tests/golden_master_audit.json and tests/golden_master_zero_dep_audit.json via update_golden_master.py: the diff is entirely previously-dropped/truncated functions now correctly recovered (e.g. matrixmultneon.s gains single-line functions C/prtstr; mainpie.s's _start recovers its full un-truncated body). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rate tri-comparison chart Regenerating the tri-comparison chart (docs/self_scan/tri_comparison_chart.svg) after the previous commit surfaced a real, measured side effect of Bug 2's own fix (no longer discarding single-line Mode A bodies): it also let through two classes of single-line NON-function matches that used to be masked by the old line-count guard, confirmed against real corpus source via the tri-comparison pipeline's own GitGalaxy-vs-ctags precision numbers: - assembly: NASM `equ` constant/data definitions collapse to one line just like a real single-instruction trampoline does (`name_segment: equ 0x1000` in `language-crucible/data/assembly/bootos/counter.asm:16`) and were being counted as functions. `assembly`'s own `func_start` regex has no visibility past the label's colon on the same line, so this is checked in `_slice_by_labels` against the already-sliced single-line block instead (`_ASSEMBLY_DATA_DIRECTIVE_RE`, gated to `primary_lang_id == "assembly"`). Measured: precision was 84.8% before #1949, regressed to 75.2% after Bug 2's fix alone, now 85.2% (115/135) with this follow-up -- net improvement over the pre-#1949 baseline, though still short of ctags' 95.0% on this language. - cobol: `SOURCE-COMPUTER.`/`OBJECT-COMPUTER.` (ENVIRONMENT DIVISION CONFIGURATION SECTION headers, never real PROCEDURE DIVISION paragraphs) collapse to one line the same way (`cics-banking-sample-application-cbsa/BNKMENU.cbl:23`). These are the same reserved-header category `func_start`'s own negative lookahead already excludes for INPUT-OUTPUT/CONFIGURATION/PROGRAM-ID/etc. two lines below -- just missing from that list. Added there instead of a detector.py check, matching the existing idiom. cobol still comfortably beats ctags on precision either way (105/148 = 70.9% vs ctags' 43.6%), so this was a correctness fix, not a badge-preserving one. agc_assembly and fortran were confirmed unaffected by Bug 2 (agc_assembly's own single-line rescues were all real per #1949's repro; fortran has no single-line-body cases in this corpus) -- no changes needed there. Regenerated docs/self_scan/tri_comparison_chart.svg and tri_comparison_ledger.json via `tri_comparison_chart.py --all --write` (required installing a local universal-ctags binary for this session -- neither apt's default package nor Ubuntu's arduino-ctags shadow-package satisfy ctags_reader.py's real-universal-ctags version check, and 4 of the 5 languages #1949 touches are ctags-only, no tree-sitter grammar, so skipping this would have silently degraded their comparison to GitGalaxy-alone). Verified via a full before/after diff of the ledger's `entries` dict: 0 missing keys, 0 validated-status changes, 0 validated-verdict text changes -- only live counts refreshed plus 5 new unrelated unvalidated discrepancy shapes surfaced incidentally (tcl, typescript, rust; left for a future tri-comparison-ledger-sweep pass, not investigated here). Regenerated both golden master fixtures again for the same reason as the prior commit (real output change, not a regression). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow-up: tri-comparison chart regeneration + a real precision regression found and fixedRegenerated assembly: NASM
Fixed with a new cobol: Full before/after diff of the ledger's Regenerated both golden master fixtures again; full test suite (7006 tests) and |
…eat ctags on precision
The tri-comparison report (tests/tools/tri_comparison_report.py) surfaced that the
previous commit's `_ASSEMBLY_DATA_DIRECTIVE_RE` fix was too narrow: it only checked
whether the label's directive appeared on the SAME single line
(`"\n" not in block`), so it caught NASM's same-line `equ` idiom
(`name_segment: equ 0x1000`) but missed the equally common GAS idiom of a data
label alone on its own line with the directive on the next
(`ape.ident:\n\t.long\t2f-1f\n...` -- an ELF note record, `language-crucible/data/
assembly/cosmopolitan/ape.S:781`; `str.error:\n\t.asciz\t"error: "`, `ape.S:1226`).
It also only recognized NASM's own directive vocabulary (equ/db/dw/...), not GAS's
dot-prefixed equivalents (.long/.byte/.asciz/...).
Fixed by widening the gap between the label's colon and the directive to
`[ \t\n]{0,80}` (bounded, ReDoS-safe -- verified via a scaling sweep, effectively
constant time up to 100k chars of non-matching whitespace) and adding the GAS
directive set. Deliberately still excludes only value-defining directives, not
alignment/metadata ones (.balign/.align/.p2align/.section/.size/.type) that a
real function can legitimately open with.
Verified every newly-excluded label against real corpus source before trusting
the fix -- all 20 are genuinely data (ELF/Mach-O/GRUB header tables, GDT
descriptors, string constants, jump/interrupt-vector tables), none are real
callable subroutines. This also corrects a wrong claim in the original #1949
PR description: `matrixmultneon.s`'s `C`/`prtstr` (cited there as "recovered real
functions") are actually `.fill`/`.asciz` data buffers -- not real functions,
correctly excluded again by this fix.
Measured via the tri-comparison pipeline (GitGalaxy vs. ctags precision on
`assembly`):
- Before #1949: 84.8%
- After Bug 2's fix alone: 75.2% (regression)
- After the first follow-up (same-line-only equ check): 85.2%
- After this fix: 93.1% (95/102) vs ctags' 78.5% (95/121) -- GitGalaxy now
beats ctags on this language; the tri-comparison chart badge flips from
ctags to GitGalaxy.
cobol and agc_assembly re-verified unaffected/still winning (105/148=70.9%
vs ctags 43.6%; 812/812=100% vs ctags 78.3%).
Regenerated docs/self_scan/tri_comparison_chart.svg,
tri_comparison_ledger.json, and tri_comparison_points_of_interest.md.
Ledger integrity re-verified via full before/after diff of the `entries`
dict: 0 missing keys, 0 added keys, 0 validated-status changes, 0
validated-verdict text changes -- only live counts refreshed. Regenerated
both golden master fixtures again for the same real-output-change reason as
the prior two commits; full test suite (7006 tests) and crucible_check.py
both pass clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow-up 2: assembly now beats ctags on precisionUsed `tests/tools/tri_comparison_report.py` (rendering the ledger's assembly entries) to find what GitGalaxy was still missing. It pointed straight at the fix: the previous commit's data-directive check only looked for the directive on the same line as the label (`"\n" not in block`), so it caught NASM's same-line `equ` idiom but missed the equally common GAS idiom of a bare label on its own line with the directive on the next (`ape.ident:\n\t.long\t2f-1f` -- an ELF note record; `str.error:\n\t.asciz\t"error: "`). It also only knew NASM's directive vocabulary, not GAS's dot-prefixed equivalents (`.long`/`.byte`/`.asciz`/...). Widened the check to a bounded This also corrects a mistake in the original PR description: Final measured precision (GitGalaxy vs. ctags on
GitGalaxy now beats ctags on assembly function precision -- the tri-comparison chart badge flips from ctags to GitGalaxy. cobol and agc_assembly re-verified still winning (70.9% vs 43.6%; 100% vs 78.3%). Regenerated the chart, ledger, and points-of-interest report again; ledger integrity re-verified (0 missing/added keys, 0 validated-status or verdict-text changes, only live counts refreshed). Full test suite (7006 tests) and |
Summary
Fixes #1949 -- two independent bugs in
_slice_by_labels(the Mode Alabel-based slicing path shared by
assembly,cobol,fortran,abap,and
agc_assembly):Bug 1 --
assembly_returnstruncated on ANY keyword occurrence, not just areal terminating statement. Fixed by:
modulo leading whitespace) before treating it as a terminator. This alone
eliminates matches embedded inside a doc-comment (
// @return dl = pc_drive...inassembly/cosmopolitan/ape.S:251) and matches that are onlya substring of a larger hyphenated identifier (
\bEXIT\bfiring insideWS-EXIT-RETRY-LOOPincobol/.../XFRFUN.cbl:105, since hyphens arenon-word characters that satisfy
\bwithout being a real boundary).END-IF/END-PERFORMfrom the shared vocabulary outright -- bothare block closers, never a real paragraph/function terminator in any Mode
A language (
cobol/.../BNKMENU.cbl:242, paragraphPMM010.).RELINToutright -- AGC's own idiom commonly uses it to open along interrupt handler, not close one
(
agc_assembly/apollo-11/AGC_BLOCK_TWO_SELF-CHECK.agc:303, routineELOOPFIN).EXIT/RETURNper-language where corpus evidence shows they'relegitimately non-terminating mid-body control flow: Fortran's
EXITis aDO-loop break (
fortran/wrf/module_sf_noahdrv.F:1999, insideSUBROUTINE SFLX); ABAP'sRETURN/EXITare an early-exit guard clause and aDO-loop break respectively (both inside method
delete,abap/abapGit/zcl_abapgit_ajson.clas.abap:192,307).Bug 2 -- single-line/blank-collapsed function bodies were silently
discarded. Removed the
len(block.splitlines()) < 2guard for thisintegration mode: single-instruction "trampoline" labels are completely
normal in assembly-family code (
agc_assembly's seven consecutiveone-instruction labels
SOPTION1-SOPTON10, zero-instruction labels inassembly/bootos/os.asm:269-270), and Mode A's "greedy to the next label"body is already a correct boundary without needing a minimum line count.
Verification
struct_func_startvsfunction_countin the realpipeline DB for the full
agc_assemblycorpus: before this fix, 52 real,ctags-corroborated labels were dropped (11.6% of 812 raw matches); after,
every file in the corpus shows zero gap.
assembly/cobol/fortran/abap -- all now show
function_count == struct_func_start.tests/extraction/languages/test_{abap,agc_assembly,assembly,cobol,fortran}{,_strict}.py:873 passed.
tests/core_engine/: 554 passed, 1 xfailed (pre-existing).tests/suite: 7006 passed, 2 skipped, 1 deselected, 11 xfailed, 3xpassed.
python tests/tools/audit_check.py: ruff/mypy/dead-key/ast-accuracy allclean against their baselines.
python tests/tools/crucible_check.py: full_precision andzero_dependency both PASS after regenerating the golden masters.
Golden master regeneration
Regenerated
tests/golden_master_audit.jsonandtests/golden_master_zero_dep_audit.jsonviatests/tools/update_golden_master.py --yes(once per venv, per repoprotocol). The diff is entirely previously-dropped/truncated functions now
correctly recovered -- e.g.
assembly/hellosilicon/matrixmultneon.sgainsthe single-line functions
C/prtstr, andmainpie.s's_startrecoversits full un-truncated body instead of stopping at the first
RET-adjacentmatch. No unrelated drift.
Test plan
languages pass
crucible_check.py) clean against regeneratedgolden masters
🤖 Generated with Claude Code