Skip to content

Comments and strings are never read as code (parsing audit 2026-10-10) - #14

Merged
tabladrum merged 43 commits into
mainfrom
fix/comment-string-masking
Oct 10, 2026
Merged

tabladrum merged 43 commits into
mainfrom
fix/comment-string-masking

Conversation

@tabladrum

Copy link
Copy Markdown
Contributor

Fixes all 53 findings of the 2026-10-10 parsing audit: places where grove read comments, docstrings or string literals as code. Many reached stored edges at 0.94–0.98 confidence. It also fixes further bugs that a new metamorphic check found on real repos. This supersedes #13, whose commit is included. It depends on provasign/astkit#4, pinned here as a pseudo-version.

Root causes and fixes

  • Seven hand-written blankers → astkit textmask.
    • The old graph blanker took 'single quoted' text for a Rust lifetime in every language, which gave wrong Python/JS/PHP call targets. It also took every # for a comment and treated \ in a Go raw string as an escape.
    • The native blanker masked strings but kept comments, which gave uses-type and extends edges from comments at native confidence.
    • The parser, graph and native layers now all mask through textmask.
  • Field and attribute maps came from a regex over the whole class text. That read other methods' locals, comments and docstrings, including numpy Attributes sections. Maps now come from indexed field symbols and class-body depth-1 masked text.
    • Java lambdas now take their enclosing method's locals by span. They used to get them only through this leak.
  • Signatures and declaration headers are parsed on masked text. This fixes Kotlin and Python bases, C# attribute strings, and Java // extends Base on the header line. Python base lists are read across lines.
  • strings.Contains on source → AST or masked facts:
    • Python abstract methods (NotImplementedError in a docstring).
    • export default.
    • declaresLocalFunction.
    • Rust inline crate paths.
  • Parser recovery path:
    • It matches and counts braces on masked text, including Go, text blocks, raw strings, heredocs and JSX text.
    • #if 0 is masked.
    • Docstrings never cross code lines.
    • C/C++ phantoms from raw strings and #define bodies are gone.
    • C#/Java call statements (new JArray(...)) are no longer methods.
    • Broken C-family files are parsed from code alone, so comments cannot move error recovery.
  • Rename plan masks the whole body (fixes PHP #[Attr], JS #field, interpolation) and COBOL rename. COBOL field reads and writes are classified per statement.
  • Fixes the decoy invariant found on real repos:
    • Import paths with comments (Rust use {} groups).
    • Rust test roots that all registered under the crate name tests; the winner changed with map order.
    • Anonymous classes resolved by name across files.
    • Java annotation skipping.

Tests

  • internal/invariance: a twin test, a clean repo vs a decoy twin, for every audit ID, run with native analyzers off and on. Plus pkg/grove tool tests (missing-implementations, rename-plan, change-impact) and parser recovery tests.
  • go test ./... passes.
  • The decoy invariant (research harness/scoring/decoy_invariant.py with astkit cmd/decoy) holds on 23 real repos across all languages.
    • The current release fails it on 7 of 8 sampled repos. On newtonsoft, comments alone move the edge count from 85k to 105k.
  • I reviewed the full-corpus graph diff against the release by sampling removed and added edges in each edge class. Details are in the session notes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KXa6Cv1ZUXcdMqXcJfQFhX

tabladrum and others added 30 commits October 10, 2026 00:38
When tree-sitter reports a syntax error, extractSymbols merges every name
the regex extractor finds that the AST missed. The Java patterns are
unanchored and the line scanner only skipped lines starting with "//", so
Javadoc prose became symbols: " * Deserializer class that can ..." in a
jackson-databind file being edited produced a class "that" spanning
lines 17-516 with 109 uses-type edges into it. Clean Guava source carried
32 such phantoms permanently (classes "for", "to", "should" spanning most
of Verify.java, "sqrt" methods from doc examples), 452 edges.

Patterns now match on a copy with comments and string literals blanked
(line numbers kept), as the C/C++ scanner already does: the C-style
blanker for Java, C#, TS/JS, Rust and PHP, and a new Python blanker for
"#" comments and (triple-quoted) strings. Signatures and bodies still come
from the source.

Guava re-index vs main: 0 syntax-recovery symbols (was 32); the only
difference is those 32 symbols and their 452 edges, no other edge added
or removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXa6Cv1ZUXcdMqXcJfQFhX
Metamorphic twin test (internal/invariance): each audit finding is a tiny
repo in a CLEAN form and a DECOY form that differs only by comments,
strings, docstrings, or layout. Both are indexed through pkg/grove's real
pipeline, normalized (file::qualifiedName#kind endpoints, no SHAs/lines),
and compared; each case also carries want/wantNot facts so equal-but-wrong
graphs fail. The table runs with native analyzers off and on; the native
run is skipped when no analyzer completed a pass (toolchain missing).

Tool-output findings (missing-implementations, rename-plan, change-impact)
are in pkg/grove; parser recovery phantoms, recovered spans, docstring
bleed, and .h language sniffing are in internal/parser.

These tests fail on this branch by design: they encode the expected
behaviour for the fixes in flight.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
stripCommentsAndStrings kept single-quoted text as code in every language
(read as a Rust lifetime), treated every # as a comment, and let Go raw
strings take backslash escapes. Every graph pass now calls maskCode, a
language-aware wrapper over astkit's textmask, which keeps offsets and line
breaks. Rename plans mask the whole body before splitting lines, so the
middle lines of block comments stay masked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
stripQuotedText blanked strings but not comments and took an apostrophe
for a string opener, so the native type-use, constructor, inheritance,
trait-use, include, module and file-import scans read comment and string
text as code (audit N1-N8), at 0.9-0.98 confidence.

- semantic.go: stripQuotedText and the per-call stripping helpers are
  gone. maskCode (textmask.Mask) runs once per symbol; typeTokensIn and
  containsTypeToken take masked text. maskedDeclHeader gives the masked
  declaration header (RawText up to the first top-level `{`/`;`).
- java/csharp: extends/implements come from maskedDeclHeader instead of
  Signature + raw first line; C# also skips leading attribute sections.
- php: file scans use textmask.MaskFile; trait `use` is read from the
  class body at depth 1 only, so method bodies (closures, comments) are
  out of reach.
- rust: impl refs, signature types, module-scope paths and `mod x;`
  scans read masked text, each body masked once.
- c/c++: #include scan reads MaskComments(MaskInactivePreprocessor(..));
  type scans read masked bodies (stripCStyleComments removed).
- go: lexical type-use pass masks the body.
- javac: package declaration read from masked source.
- Removed dead text helpers: javaQualifiedCalls, phpStaticCalls,
  cFamilyQualifiedCalls, cFamilyContainsCallable, containsCall.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Base lists, uses-type names, nested-function checks, creation sites, Rust
path pins and crate refs, Go embeds and callable JS fields were read from
raw Signature/RawText, so comments, docstrings and string literals made or
hid edges.

- codeSignature masks a signature by aligning it with RawText, so headers
  the extractor collapsed onto one line (Python and Kotlin class headers)
  keep the code after a `#`/`//` comment. declHeader replaces every
  Signature-or-first-line base-list read (TS/JS/Java, C#, PHP, Swift,
  Kotlin, ObjC; extends edges and change-impact declared supers).
- Python bases come from the whole class header, paren-aware, so a
  multi-line or commented base list keeps every base; keyword arguments
  are dropped.
- buildUsesType scans the masked signature, and only links types in a
  compatible language (a Kotlin signature no longer reaches a Python
  class in the same directory).
- declaresLocalFunction, creationSite and rustPinByPath read the caller's
  masked body (masked once per caller); buildRustInlineRefs,
  goEmbeddedTypes and graphCallableSymbol mask their text.
- maskCode lexes COBOL symbol text as free format: the parser has already
  cut off the sequence and indicator areas.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pythonAbstractMethod treated any method whose text mentioned
abstractmethod or NotImplementedError as abstract, so a docstring saying
"never raises NotImplementedError" made missing-implementations list
every subclass. Abstract is now an abstractmethod-family decorator, or a
body whose first statement after the docstring is raise
NotImplementedError, read from masked text. The contract-body and
abstract/default signature checks read masked text too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Field reads and writes were classified per physical line with only quotes
removed: an inline `*> WAS WS-OLD` comment made a write edge, and
`MOVE A\n TO B.` read B instead of writing it. The body is now masked and
split into statements that run to the next verb or separator period.

The mainframe rename plan counted matches on the masked line but replaced
on the raw one, so a name inside a `*>` comment on an edited line was
renamed too. It now replaces at the masked match offsets only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every tests/, benches/ and examples/ root registered as a crate named after
its directory, so all of them shared the name "tests" and the map
iteration chose which one won. A library's own `mod tests` paths
(`tests::TempDir`, `crate::tests::...`) then pulled that integration-test
directory into the library's scope: on ripgrep the ignore crate reached
tests/ on one run and crates/ignore/tests on another, with uses-type edges
into test helpers. Test, bench and example targets are no longer
registered by name, and names are registered in sorted root order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Field/attribute maps no longer regex the whole class RawText (method
locals, commented-out declarations and docstrings read as fields):
- classBody (localtypes.go): masked class body reduced to its own member
  declarations (brace depth 1; Python: body-indent statements), memoized
  per class on edgeIndex. Used by Java (javaClassFields: indexed field
  symbols first, then classBody), C# csharpFieldRe, C++ cppLocalDeclRe,
  PHP phpPropertyRe, TS tsCollectionFieldRe, Go goStructFieldRe.
- Python pyClassBodyAttrs: class-body annotations/class refs only at body
  indentation, in code (never docstrings); string annotations kept.
- PHP phpTraitRulesFor: only depth-1 `use` statements of masked text.
- javaIndexedFieldType masks annotation strings.
Per-call-site raw-line reads use maskedSymbolLines (memoized):
tsReceiverChainAt (own line first; window/whole body only on a unique
match), javaCallResultOwners, phpNarrowNewByNamespace, csRecoverArgs,
pyCallHasSplat. The pyLocalTypes `self.<name>` guard reads masked code.
tsDeclParams(lang, text) finds the paren group on masked text;
splitParams splits on masked text (L10).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sniffHeader matched its C++/ObjC markers on raw text: a block-comment
line starting with "namespace" or "class", an #if 0 branch, or the
C-library idiom `#ifdef __cplusplus / extern "C" { / #endif` made a C
header C++. Markers now match on text with comments and #if 0 branches
masked (astkit textmask) and the C++-only branches of __cplusplus
conditionals blanked. MemberOccurrences parses a .h in the language the
indexer chose instead of astkit's own sniff (astkit cppHeaderMarker has
the same comment/guard problem).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
blankJSComments was a hand-written lexer that read a regex literal such
as /[/*]/ as a block comment opener and blanked every export clause
after it. Export clauses now match on textmask.Mask output; the module
string after `from` is read from the original source at the same offset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replaces the parser's hand-written blankers (blankCFamilyComments,
blankPythonCommentsAndStrings, stripCppCommentsAndStrings) with astkit
textmask (audit 2026-10-10, P1-P17):

- Regex recovery (all languages, Go included) matches on scanMask:
  MaskFile (PHP inline HTML, heredocs, raw strings, text blocks,
  template literals, Python continued strings) plus
  MaskInactivePreprocessor; C/C++ also blank directive lines with their
  continuations (multi-line #define bodies). Signatures and bodies still
  come from the source; brace/indent counting (extractBraceBody,
  extractIndentBody, bracesBalanced) and export keywords use the mask.
- Recovery in JS/TSX also blanks jsx_text by node range from the partial
  tree (textmask lexes JSX text as code).
- The AST parse of C/C++/ObjC/C# sees #if 0 / #if false branches masked.
- precedingCommentBlock walks only through comment lines (comment-masked
  comparison): a code line with a trailing comment, or a string holding
  "/**", is never a docstring; plain /* */ blocks are not doc comments.
- cppFileNamespaceBindings reads the masked file; enrichCppNamespaces
  matches qualified names on the masked Signature; cFamilyMarkPrototype
  masks RawText.
- jsDefaultExportAt reads masked lines; import alias patterns read the
  masked import statement; fallback extractImports masks too.
- tsExtendOverloadSpans takes overloads from the tree: a bodiless
  function/method signature declaring the same name, not any line ending
  in `;` (a call `foo(1);` widened foo).

P16 twin fix: the decoy's `foo(1);` is a real call, so the clean form now
makes the same call after the function; both forms carry the same
<top-level> -> foo edge and the case still fails on the old code.

ExtractorVersion bumped: extraction output changes for unchanged files.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ead masked code

Base-list, typealias, field, parameter and return-type parses use
codeSignature / masked firstLine(RawText) instead of the raw Signature.
memberclassify: C macro container_of/cast scan, C return-type and field
signature regexes, and the Python module constructor regex run on masked
text.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A '<' or ')' inside a default string stopped the split in change-impact
parameter typing and arity counting.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A comment after the header colon is folded into the stored signature;
LastIndexByte(')') then reached a ')' inside the comment and the MRO walk
lost every base, dropping ~700 inherited-method calls on flask under the
decoy invariant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Astkit takes a grouped import's path from source, so a comment inside
'use std::{ ... }' became part of the import string and of the crate heads
parsed from it; ripgrep's crate scopes shrank under the decoy invariant.
The pub-use branch also rebuilt the path from the raw text.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
'new JArray(1, 2),' and 'return Build(x);' matched the method pattern with
the keyword as a return type; newtonsoft's recovered tests carried ten
phantom methods named after callees.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A lambda is parented to its class, so its receivers ('() -> cache.get(k)')
were typed only through the class field map, which used to regex every
method body and so carried every local. With field maps limited to real
fields, lambdas lost ~22k guava call edges; they now take the enclosing
method's parameters and typed locals, found by span.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
javaDeclSource skipped leading annotations with its own quote tracking, so
a comment inside an annotation's argument list (an apostrophe or a paren
in it) ran the annotation to the end of the text. The method lost its
parameter list and the override edge to its abstract declaration went
with it (decoy invariant, jackson-databind BuilderCreatorSubtype4742Test).
Find the end of the annotations on textmask output and return the
original text from that offset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`Map<K, V> map;` assigned later in a try gave map no type, so every call
on it dropped as an untyped (library) receiver. Its type used to arrive by
accident, from javaFieldRe run over every method body of the class; since
the field map reads only the masked depth-1 class body, the local
declaration regex has to end on `;` too. guava: about 12k dispatch edges
from MapInterfaceTest-style tests came back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Anonymous classes are named by position (`<anonymous@193:9>`), unique only
inside a file; guava has 4,308 of them under 2,289 names. The class
hierarchy (subclassOverrides), interface method sets
(buildInterfaceSatisfaction) and base walks from a method's own class
(javaLocalTypes, inherited/super calls) looked them up by name, so one
file's anonymous class took another's base or pooled its methods. A
comment line above either moved the collision: about 10k guava dispatch
edges changed under the decoy invariant.

baseClassesInFileFor reads a file-local name's bases from its declaration
in the caller's file; subclassOverrides keys such a class by name and
file; method sets key it by file instead of directory.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
javaLocalTypes and javaArgTypes split a parameter on whitespace, so
`Table<R, C, V> table` and `Map<? extends K, ? extends V> map` came apart
and the parameter went untyped; javaParamTypes already collapsed the
spaces inside angle brackets first. These parameters used to be typed by
accident, through a class field map that read every method body's locals
(guava TableCollectors.mergeTables, TableCollectionTest).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`result.values.size()` with `Map<String, Integer> values` on result's
class resolved the chain to Map, found no Map in the index and gave up.
It used to work by accident: the outer class's field map read the nested
class's fields, so `values` looked like a field of the caller's own
class. narrowByChainType now resolves the last hop as narrowByLocalType
does for a receiver of that type (subclass overrides, base walk, fan-out
cap). Java only; TS keeps its behavior.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tabladrum and others added 13 commits October 10, 2026 12:44
astkit now cuts attribute sections out of C# signatures (strings in them
read as types), so buildUsesType lost the types attributes refer to:
typeof(StringEnumConverter), MemberSerialization.OptIn,
NullValueHandling.Ignore (newtonsoft: ~200 true edges). Read them from the
symbol's Annotations, masked: each attribute's class (XAttribute when the
index has it, as C# binds it; JsonProperty used to bind the unrelated
Serialization.JsonProperty class), every typeof(T), and the type of a
member-access value. Strings, comments and named-argument names
(`Name = "x"`) are not types.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cFamilyParamTypes falls back to a symbol's body when its signature has no
parameter list (doctest's `struct DOCTEST_INTERFACE AssertData`,
recovered as a function). The first `(` of the raw body could be in a
comment, so a comment line decided whether `str` was a parameter and
whether `content(str)` passed a function named str (nlohmann/json decoy:
one calls edge only in the decoy). Signature and body are masked first;
the body is masked once and reused for the local declarations.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- extractBraceBody gave up 500 physical lines into a body; masked comment
  lines counted, so comments moved where a recovered class was cut and
  what the scanner read after it (fmt gtest.h). It now counts code lines.
- enrichCppNamespaces chose the narrowest enclosing namespace by line
  count. Recovered spans can overlap without nesting (`namespace testing`
  10417-11159, `namespace internal` 10418-11161 in gmock-gtest-all.cc),
  and a comment line elsewhere in either span flipped the choice. The
  closest scope is now the later opening, then the earlier close: the
  same answer for nested scopes, and line-count free.
- The C++ class pattern skips an export macro before the name
  (`class GTEST_API_ RE {` was a class named GTEST_API_).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tree-sitter's error recovery prices every byte and line it skips,
comments included, so in a file the grammar could not parse a comment
line changed which declarations survived: AFNetworking's
AFURLSessionManager.m flipped between two ERROR soups (`if` functions
included), fmt's gtest/gmock headers and nlohmann's json_v3_10_5.hpp and
doctest.h lost or gained hundreds of symbols, and includes inside ERROR
regions came and went.

- parseBrokenCFamily (commentcut.go): a C, C++ or Objective-C file whose
  first parse has errors is re-parsed with every comment cut to one space
  and comment-only lines dropped, so the parser sees the same bytes for
  any commenting of the program. The #else-blanked alternative is tried
  on that input as before (now for Objective-C too). commentCuts.restore
  maps spans and call/attribute lines back and re-cuts bodies from the
  source, comments included. A clean parse is unchanged. Includes of a
  broken file are read the same way.
- blankCFamilyMacroLines now runs for Objective-C and blanks more macro
  uses the grammar cannot read, which is what threw these files into
  whole-file ERRORs: macro statements at file or namespace scope
  (`GMOCK_DEFINE_DEFAULT_ACTION_FOR_RETURN_TYPE_(float, 0);`), a trailing
  attribute call after a declarator (`... *metrics AF_API_AVAILABLE(...);`,
  also before a method body), `NS_DESIGNATED_INITIALIZER;` ending a
  method, `typedef NS_ENUM(...) {...};` (astkit reads it from text), an
  export macro between a class key and its name, a specifier macro after
  `static`/`friend`/... or before `Type name`, and member-declaring macro
  lines in class bodies. It tracks namespace/class/other brace scopes.

Decoy invariant: AFNetworking, fmt, nlohmann/json and jansson all OK.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	internal/invariance/twin_test.go
enrichRecoveredClassParents picked the narrowest enclosing class span.
Recovered spans can overlap without nesting, and there the line counts
decide, so a comment line inside one span flipped the parent. Use the
rule enrichCppNamespaces uses: the class that opens later, then the one
that closes earlier (the same answer for proper nesting).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… typed generics

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… starved

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tabladrum
tabladrum merged commit 2735da8 into main Oct 10, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant