fix(assembly): strip /* */ block comments; re-validate 16-folder tri-comparison corpus - #2524
Merged
Merged
Conversation
…der corpus The assembly crucible corpus grew 3->16 folders (253 new files, PR #13) since the last tri-comparison investigation (2026-08-27, 3 folders). Re-ran the full gather-and-diff per the tri-comparison-ledger-sweep skill's own guidance for a substantially-grown corpus rather than trusting the already-`validated` status. Real confirmed engine defect found and fixed: assembly's lexical_family ("line_exclusive") only ever recognized `;`/`#` line comments -- but `.S` files are routed through the C preprocessor and routinely carry genuine `/* ... */` block comments (BSD/FreeBSD kernel license headers, Emacs modelines, register-usage doc comments) that were never stripped at all. func_start matched label-shaped text INSIDE the unstripped comment: `Result:` (linux_1_0_kernel's stack-layout doc comment) and `r9:`/`r10:`/`r11:` (freebsd_kernel_arch's register-usage doc comment), 5 occurrences corpus-wide. Fixed with a new `_strip_asm_block_comments` prism.py pre-processing pass, run BEFORE `;`/`#` line-stripping -- deliberately, confirmed by direct corpus measurement rather than assumed: 121 real `/* ... */` blocks in this corpus contain a bare `;`/`#` internally (copyright prose, URLs, Emacs modelines like `/*-*- mode:unix-assembly; indent-tabs-mode:t; ... -*-*/`), so stripping line comments first would truncate every one of those blocks at its first internal `;`/`#`, corrupting the search for the block's real closing `*/`. The reverse risk (a `;`/`#` comment containing an unclosed `/*`) was checked and occurs zero times in this corpus. Same shielded-alternation idiom as the generic REGEX_MATRIX stripper (LITERAL_MASK_PATTERN tried first, so `.ascii "a /* b"` passes through unharmed) and the same ReDoS-accepted `/\*.*?\*/` shape every "standard_block" C-family language already uses. 4 new regression tests (basic strip, the line-comment-ordering hazard, literal-shielding, ReDoS). Verified: exactly 5 fewer func_start matches corpus-wide post-fix (1144 -> 1139), matching the confirmed count precisely. Both golden masters re-blessed -- the diff is large but fully expected (removing 5 phantom "functions" shifts per-directory documentation/tech-debt/cognitive-load averages for the affected folders, and the whole corpus's 3D topological layout reflows globally the same way any structural-mass change does). Both assembly tri-comparison ledger shapes re-validated at ~70x the prior sample size (2899 total slots vs. the old corpus's much smaller one) -- same disposition as before (no credit/debit on the ctags-only shape; credit gitgalaxy on the gitgalaxy-only shape), but with two newly-confirmed mechanisms behind the much bigger ctags-only count: ctags' generic Asm parser tagging a bare macro- invocation keyword as a literal "label" (NASM's own `%macro x 1+.nolist` instruction-set self-test files, tagged up to 192 times in one file; cpm65_6502's `zproc`/`zendproc`/`.label` procedure-macro convention, same mechanism), and tagging ordinary instruction mnemonics as repeated "labels" on FASM-dialect files (raspberrypi_baremetal's `format`/`include`/`code64` directives desync ctags' line-start heuristic entirely). GitGalaxy correctly excludes all of these -- func_start requires a real `name:` declaration head none of them have. One new confirmed GitGalaxy-correct mechanism credited: GAS's `funcname.localname:` dot-scoped local-label convention (do_e820.jmpin etc.), which ctags structurally can't parse -- same "GitGalaxy correct, ctags can't" bucket as the existing NASM-local-label precedent. One residual, deliberately-not-chased false positive documented in the verdict (a single `#define SYSCALL(name) ... name: ...` C-preprocessor macro template misread as a real label -- narrow, one occurrence, disproportionate to fix). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014BKXKcW29F3pTm7To2wMwS
…parison-validation # Conflicts: # docs/self_scan/tri_comparison_ledger.json # docs/self_scan/tri_comparison_points_of_interest.md
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| return pattern.sub(_repl, text), lits | ||
|
|
||
| return self._PS_HERESTRING_RE.sub(_repl, text), lits |
Contributor
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
2 participants
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
Re-validated assembly's tri-comparison ledger against the corpus's real growth (3→16 folders,
253 new files, PR #13) — the existing
validatedstatus was only ever checked against 3 folders.Found and fixed one real GitGalaxy engine defect; confirmed the corpus's dramatic apparent
"recall drop" is almost entirely a ctags-side measurement artifact, not a GitGalaxy problem.
The real fix:
/* */block comments were never stripped for assemblyAssembly's
lexical_family("line_exclusive") only recognized;/#line comments — but.Sfiles are routed through the C preprocessor and routinely carry genuine
/* ... */blockcomments (BSD/FreeBSD license headers, Emacs modelines, register-usage doc comments). Confirmed
real false positives:
func_startmatched label-shaped text inside an unstripped comment —Result:(a stack-layout doc comment) andr9:/r10:/r11:(a register-usage doc comment), 5occurrences corpus-wide.
Fix: a new
_strip_asm_block_commentsprism.py pre-processing pass, run before;/#line-stripping — deliberately, confirmed by direct measurement, not assumed: 121 real
/* */blocks in this corpus contain a bare
;/#internally (copyright prose, URLs, Emacs modelines).Stripping line comments first would truncate every one of those blocks at its first internal
;/#, corrupting the search for the real closing*/. Checked the reverse risk too (a;/#comment containing an unclosed/*) — zero occurrences in this corpus. Sameshielded-alternation idiom the generic stripper already uses, same ReDoS-accepted
non-greedy-bounded-by-two-delimiters shape every
standard_blocklanguage already relies on.Verified: exactly 5 fewer
func_startmatches corpus-wide (1144 → 1139), matching precisely.Golden masters re-blessed (large diff, fully expected — removing 5 phantom functions shifts
per-directory metrics and the whole corpus's 3D layout reflows, same as any prior corpus-mass
change).
The bigger picture: ctags' Asm parser is very unreliable on this corpus, not GitGalaxy
Raw numbers before investigating looked alarming — GitGalaxy recall dropped to ~38% against
ctags' ~99%. Investigated at scale (15+ concrete file:line citations across nearly every new
folder) before trusting that number. It's a measurement artifact:
self-test suite wraps opcodes in a
%macro x 1+.nolisthelper invoked as barex OPCODE ...(no colon anywhere) —
ctags -xtags the wordxas a label up to 192 times in one file.cpm65_6502's
zproc NAME/zendprocprocedure-macro convention gets the identical treatment(
zproc/zendprocthemselves tagged, not the real names).raspberrypi_baremetal's
format/include/code64directives desync its line-start heuristicentirely, tagging
mov/add/ldr/mrc/etc. as repeated "labels."excludes) also just scales up proportionally with the bigger, more diverse corpus.
GitGalaxy correctly excludes all of these —
func_startrequires a realname:declarationhead none of them have.
One new confirmed GitGalaxy-correct mechanism, credited: GAS's
funcname.localname:dot-scoped local-label convention (
do_e820.jmpinetc.) — ctags structurally can't parse it (0/6tagged, confirmed via raw ctags run), GitGalaxy finds all of them. Same bucket as the existing
NASM-leading-dot-local precedent.
One residual, deliberately not chased false positive, documented in the ledger verdict: a
#define SYSCALL(name) ... name: ...C-preprocessor macro template wherenameis a parameter,not a real label — one occurrence in the whole corpus, narrow root cause, not worth the
engineering effort relative to its impact.
Verification
string-literal shielding, ReDoS) — all passing alongside the full
core_engine(576) +extraction(231 assembly-specific) suites.crucible_check.py: clean on bothfull_precisionandzero_dependencyafter re-blessing bothgolden masters.
audit_check.py: clean.mainconfirms the ledger's credit/debit/status is unchanged for bothassembly shapes (verdict text enrichment only) — no other language touched.
🤖 Generated with Claude Code