[mpapis]

Follow-up to #401, taking the one point the maintainer left open there:

"The nit about moving emitCapturedMixinBase into src/ingest_ruby.h behind an explicit language hook is still open. Take it if you'd like, as a small follow-up PR; otherwise we'll fold it into a later train."

No behaviour change; byte-identical against the merged main.

Point 1 — make the language seam explicit

@reference.mixin is a language-neutral capture name, and the consumer arm spends no Lang:: branch to reach it — correct. But emitCapturedMixinBase was entirely Ruby (Ruby fields, Ruby directive names, Lang::Ruby refs) while sitting in the shared ingest_relations.h. A second language that adopted the capture would have been fed to Ruby's guards and silently dropped.

• The reader moves to src/ingest_ruby.h as emitCapturedRubyMixinBase — the include order allows it (relations → ruby → sidecap).
• The language now arrives from the call site (le.lang) and is asserted inside: EXPECTS( lang == Lang::Ruby ).
• queries/ruby/tags.scm and the consumer comment now state that Ruby is the capture's only emitter today, and name what a second language must add: its own reader plus a per-language dispatch at the arm — never Ruby's guards. PHP's in-body use Trait; is cited as the documented sibling.

Also folds #401's non-blocking points 2–4

These are accuracy fixes to comments and a test that #401 itself put on main.

Point 3 — six stale / anachronistic comments. graph.h, ingest_cache.h's 131 history entry, model.h and resolve.h named the new reader as the parser-version-131 mechanism. History lines now keep captureRubyMixinBases and add "since 173: …". rubyinheritcheck.sh:29 and legotransitivecheck.sh:21 named the retired function as current; both corrected. rubyMixinTargets' header implied the capture reads the constant's byte through it — it does not call it; now stated.

Point 2 — the one non-byte-identical construction. The Include record still comes from the captureIncludes walk, capped at kMaxImportContainerDepth (256 AST levels), while the query-borne inherit reference has no such bound, so past that depth the reference has no directive to join to. Pathological and disclosed; a sentence in the CHANGELOG and the helper comment makes "byte-identical" exact. (I could not re-derive it against 1bcb6540 — that base already contains #401 — so I cite the maintainer's measurement rather than claim my own reproduction.)

Point 4 — sharpen the arm. Its text-guard negative Logger.new( MixA ) has a receiver, so the receiver half of rubyNamedDirective refused it too — a change that dropped only the name comparison would still have passed. Added a receiverless class-body raise MixA (refused by the name guard alone, 0 refs) and extend/prepend lines, so one count covers all three directives: 4 → 6.

Point 5 — left in, flagged

The grep guard that pins the capture's presence and the retired definition's absence stays (it is the only thing that catches "the old walk was restored"). #401 noted keeping-or-dropping it is the maintainers' call; I have not dropped it. Say the word and it goes.

Verification

• Byte-identical vs main (1bcb6540): map, --deps --limit=100000, --skipped, and --callers/--callees/--impact/--uses over the Rails corpus, the mixin fixture and the Ruby fixtures — empty diff.
• rubyinheritcheck (arm now reads 6), legotransitivecheck, rubyreachcheck, chacheck, rubyclassrecvcheck, rubyattrscheck, rubyrakejbuildercheck — ALL PASS.
• --quality-delta=origin/main..HEAD gates 0. One non-gating new-symbol row: emitCapturedRubyMixinBase at 6 params vs a bar of 5 — the direct cost of taking lang from the call site, which is what point 1 asked for; new-symbol rows never gate.
• ASan rebuild + smoke on the repo, the Rails app and both fixtures.
• kParserVer unchanged — no extraction semantics move.

Authored by @mpapis with AI assistance (DeepSeek V4.1 Flash / Qwen 3.8 Flash) under his supervision.

Summary by CodeRabbit

• Bug Fixes
  • Improved Ruby mixin inheritance handling, including extend, prepend, and receiverless raise cases.
  • For deeply nested modules beyond the 256-level capture limit, query-captured inheritance references may appear without a matching include record. In a 260-level nesting test, --lego now reports an implementor and marks the result as unscoped.
• Documentation
  • Clarified Ruby mixin support and its behavior across parser versions.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Repository: redhat-et/ripwire/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 58b8a407-5592-43df-9043-88b6fb5747f4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: redhat-et/ripwire/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 8011d15f-24a6-4194-9d91-5f8d02455952

📥 Commits

Reviewing files that changed from the base of the PR and between 1bcb654 and c1824cc.

📒 Files selected for processing: 11

• CHANGELOG.md
• queries/ruby/tags.scm
• src/graph.h
• src/ingest_cache.h
• src/ingest_relations.h
• src/ingest_ruby.h
• src/ingest_sidecap.h
• src/model.h
• src/resolve.h
• test/legotransitivecheck.sh
• test/rubyinheritcheck.sh

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Ruby mixin inheritance references now use a Ruby-specific emitter for the @reference.mixin capture. The tests add directive and rejected-call cases. The changelog records an exception to the include-depth byte-identity claim and a parser-version change.

Changes: Ruby mixin capture

Layer / File(s)	Summary
Capture and Ruby-specific emission queries/ruby/tags.scm, src/ingest_relations.h, src/ingest_ruby.h, src/ingest_sidecap.h, src/graph.h, src/ingest_cache.h, src/model.h, src/resolve.h	The @reference.mixin capture is routed to emitCapturedRubyMixinBase. The emitter checks the Ruby directive and allowed position before emitting a base reference. Comments describe the updated capture path.
Directive cases and expected results test/rubyinheritcheck.sh, test/legotransitivecheck.sh, CHANGELOG.md	The Ruby inheritance fixture adds extend, prepend, and raise cases. The expected reference count changes from 4 to 6. The changelog documents the include-depth exception and parser-version change.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Refactor · Severity of issue fixed: Low

Suggested reviewers: barefootski

Merge Risk: ⚪ Minimal · up to c1824

No supported merge-blocking issue remains in the reviewed changes.

Pre-merge checks: 4

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main change: making the Ruby mixin capture’s language seam explicit through a refactor.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

_{Comment @coderabbitai help to get the list of available commands.}

[joyful-ii-V-I]

Thanks @mpapis — this is exactly the follow-up we asked for, and it checks out.

What we verified

• No behaviour change. Our byte-identity harness (test/langidentitycheck.sh, base 1bcb6540 vs this head) over the Ruby and Python fixture corpora plus a real Ruby repo and a real Python repo: 294 argv, 294 SAME, 0 DIFF, 0 ERROR; the shared-file == Lang:: ratchet stays at 352 (the new EXPECTS( lang == Lang::Ruby ) is in ingest_ruby.h, a per-language file). 28 more --lego/--deps/--tree runs, including the 260-level fixture, differ only in the moved notice's source line number.
• The seam is the shape we suggested. The reader lives in src/ingest_ruby.h, the language comes from the call site, the contract is asserted, and the comments at tags.scm and the consumer arm say that Ruby is the only emitter today and what a second language has to add. --callers shows one call site.
• Point 4 is a real improvement. We made a mutant that drops only the name half of the directive guard. Your arm fails on it (expected ruby_bases_unscoped="6" … got "7", from the new raise MixA line). The previous arm passes the same mutant. Your arm also passes on the base binary, so the change is test-only.
• No kParserVer bump is needed: the ingest_cache.h and tags.scm hunks only change comments (qschemetripcheck and qextractionkeycheck are green). The touched gates (rubyinheritcheck, legotransitivecheck, chacheck, rubyreachcheck, rubyclassrecvcheck) are green on base and head, and green under ASan too. --quality-delta gates 0. Its one row, the 6-parameter new symbol, is the expected cost of passing lang.

One wording fix before merge (our mistake, not yours)

The new sentence about the 260-level construction, in both the CHANGELOG and the emitCapturedRubyMixinBase comment, says the answer then carries ruby_bases_unscoped="1". The comment adds "a second degraded-notice line". We re-ran it on this head and on main with the original fixture (260 nested modules, include DeepMix innermost). Here is what we got:

--lego=DeepMix  ->  implementors="1"  <impl n="N259" …/>
stderr          ->  one line: "import-container nesting past the depth bound — deeper imports not captured"
ruby_bases_unscoped  ->  absent on every verb (map, --tree, --deps, --lego, with or without --legend=full)

So the "gains one implementor" half is right. The attribute and the second notice come from our #401 note, which we measured on #401's pre-merge head, and they don't reproduce on the merged code. Something like this would match what happens:

…so past that depth (260 nested modules with an include M innermost) --lego lists the includer as an implementor, while the depth-bound notice says deeper imports were not captured. No real code nests 256 levels deep.

Please change the code comment the same way. You can keep or drop the point-5 grep guard; we'd keep it.

[mpapis]

Corrected in e13a2afa — both sites, your wording.

The disclosure claimed the deep construction carries ruby_bases_unscoped="1" and a second degraded-notice line. It does not, on main or this head. I reproduced your fixture independently (260 nested modules, include DeepMix innermost) and got exactly what you measured:

• --lego=DeepMix → implementors="1" with <impl n="M259" — the "gains one implementor" half was right, so it stays;
• stderr → a single [math degraded] ingest: import-container nesting past the depth bound — deeper imports not captured;
• ruby_bases_unscoped → 0 occurrences across map, --tree, --deps, --lego and with --legend=full.

The CHANGELOG sentence now reads as you wrote it, and the helper comment names the notice it quotes and says the attribute appears on no verb. (Credit where due: I saw implementors="1" and no attribute when I first tried to reproduce this in #401, which is exactly why I cited your measurement instead of claiming my own — I should have gone and checked rather than leaving the attribute in the text.)

Point 4: the mutant test is the nicest confirmation of that arm I've had — that dropping only the name half now turns raise MixA into a 7th reference and fails the count, where the old arm sailed through.

Point 5: guard kept, per your preference.

Tripwires stay green — qschemetripcheck and qextractionkeycheck both PASS, confirming the ingest_cache.h/tags.scm hunks are comment-only and no kParserVer bump is needed. rubyinheritcheck (arm still reads 6), legotransitivecheck, chacheck, rubyreachcheck, rubyclassrecvcheck, deckcheck, docdriftcheck all PASS, and --quality-delta still gates 0 with the single expected 6-parameter new-symbol row.

Authored by @mpapis with AI assistance (DeepSeek V4.1 Flash / Qwen 3.8 Flash) under his supervision.

[joyful-ii-V-I]

Thanks. e13a2af fixes the one open point (S1): the CHANGELOG entry and the helper comment now describe what the merged code does for the 260-level construction. Your independent reproduction matches ours. The seam is the shape #401 asked for, and the change remains output-neutral.

CI on acfb42b: every job passes except memguardcheck B12 on one macOS Release shard. That is a known intermittent failure, unrelated to this change.

This is ready. One step remains before it can merge: the integration train in #415 is about to land, and it touches the same part of CHANGELOG.md. Once #415 is on main, please merge origin/main into this branch again (the only conflict should be CHANGELOG.md). We'll merge it after that run is green.

[joyful-ii-V-I]

#415 (the integration train) is now merged into main. When you have a moment, please merge origin/main into this branch. CHANGELOG.md should be the only conflict. We will merge once that CI run is green.

[joyful-ii-V-I]

Thanks for the re-merge. I re-checked the commits after the first review:

• e13a2af: the S1 wording fix (CHANGELOG + the ingest_ruby.h comment only).
• acfb42b and 0246364: merges of origin/main. The diff against main is the same 11 files as before; the CHANGELOG hunk only moved.
  This merges once CI on 0246364 is green.

[joyful-ii-V-I]

Merged. Thanks @mpapis: this makes the Ruby mixin seam explicit and corrects the comments, and the turnaround on the review was quick.