Skip to content

refactor(ingestion): share a codec for __heritage__/__property__ markers (Ruby + Dart) (#1994)#2007

Merged
magyargergo merged 2 commits into
mainfrom
fix/1994-shared-marker-codec
Jun 4, 2026
Merged

refactor(ingestion): share a codec for __heritage__/__property__ markers (Ruby + Dart) (#1994)#2007
magyargergo merged 2 commits into
mainfrom
fix/1994-shared-marker-codec

Conversation

@magyargergo

Copy link
Copy Markdown
Collaborator

Stacked PR 5/5 — base fix/1991-ruby-mixin-module-trait (#2006). Auto-retargets when #2006 merges.

Summary

Maintainability follow-up to #1978 / #1982. The Ruby (and, independently, Dart) heritage/property pipeline encoded side-effect facts as ':'-delimited synthetic-import marker strings, constructed and parsed by hand at ~8 sites with the field layout kept in agreement only by a comment — the structural fragility that produced the #1981 edge-drop. This routes every site through a single shared codec with no behavioral change (the ':' wire format is preserved byte-for-byte).

What changed

New gitnexus/src/core/ingestion/utils/heritage-marker.ts:

Routed through it:

  • Ruby — construct (captures.ts), parse (scope-resolver.ts), and the duplicated guard literals in interpret.ts + import-target.ts.
  • Dart — construct (captures.ts) + parse (scope-resolver.ts). Dart already single-sources its prefix and is heritage-only, so its import-target guard is left untouched (no invented __property__ path).

Language-neutral: the codec keys only on the literal shared prefixes — no provider branching.

Verification

  • New codec unit test: round-trip, colon-throw, prefix bytes, isHeritageMarker parity (5 cases).
  • ruby resolver suite + ruby-captures-golden: registry-primary 155/155 with the golden unchanged (zero diff) — proving the ':' wire format is byte-identical; legacy green.
  • dart resolver suite: registry-primary 63/63; legacy green.
  • tsc --noEmit clean; prettier clean.

No new graph edges or behavior — pure refactor.

Closes #1994.

🤖 Generated with Claude Code

@vercel

vercel Bot commented Jun 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gitnexus Ready Ready Preview, Comment Jun 4, 2026 10:44am

Request Review

@magyargergo

Copy link
Copy Markdown
Collaborator Author

Tri-review — approve-with-nits

Method: per-PR multi-lens review → independent adversarial re-verification → static-analysis alignment (tsc/eslint/prettier, all clean) → cross-stack synthesis + critic gate.

Review: PR #2007 — shared __heritage__/__property__ marker codec (#1994)

Verdict: Approve with nits. This is a clean, genuinely behavior-preserving refactor. The findings are all minor/nit maintainability points — none block merge.

What I verified

  • Byte-identical wire format (CONFIRMED). The ruby-captures-golden test snapshots a SHA-256 digest over tag|text|range for every capture, including @import.source (the encoded marker payload), across fixtures that exercise include/extend/prepend/attr_accessor (ruby-app, ruby-qualified-mixin, ruby-nested-mixin-*, ruby-write-access). A zero-diff golden therefore genuinely proves encodeMarker emits the same string the old `${PREFIX}${a}:${b}:${c}` template did — this is a positive byte-identity guard, not a dangle-only check.
  • decodeMarker / isHeritageMarker are 1:1. decodeMarker mirrors the historical slice(PREFIX.length).split(':'); the Dart resolver's new decoded?.kind !== 'heritage' filter is equivalent to the old startsWith(DART_HERITAGE_PREFIX) (a __property__: raw is skipped in both). isHeritageMarker is the literal prior startsWith('__heritage__:') || startsWith('__property__:') pair (both prefixes carry the trailing colon — verified).
  • The colon-throw is unreachable on the real path. Every caller pre-normalizes: Ruby ownerName/mixinName flow through splitQualifiedName(...).join('.') (::.), propName strips the leading symbol colon and only matches simple_symbol, callName is a Ruby method ident; Dart fields are identifiers/type_identifier. So the throw converts the fix(ingestion): fully-qualified nested-type identity for C++/Ruby — structure (#1978) + resolution (#1982) #1981 silent drop into a loud failure with no real-ingestion crash risk.
  • Worker ≡ sequential parity is structural. encodeMarker/decodeMarker sit on the shared provider.emitScopeCaptures + scope-resolution path that both the worker (≥15 files, parse-worker.ts:856) and sequential (parsing-processor.ts:480) legs funnel through — no leg-specific code was added, so no leg-specific test was needed.
  • Combined refactor(ingestion): share a codec for __heritage__/__property__ markers (Ruby + Dart) (#1994) #2007-on-fix(ingestion): qualify Ruby same-tail nested mixin modules + route IMPLEMENTS by scope (#1991) #2006 merge is correct. In ruby/scope-resolver.ts, decodeMarker is wired into both the heritage loop (:86) and property loop (:129), and fix(ingestion): Ruby same-tail nested mixin-MODULE (Trait) node collision — structure phase never qualifies module ids (#1982 / #1978 follow-up) #1991's qualifyMixinByOwnerScope (:20, called at :98) is preserved intact.
  • tsc --noEmit clean; CHANGELOG.md untouched; goldens additive (zero-diff). imp.targetRaw is typed readonly string, so the Ruby decode path is type-safe.

Findings

Minor

  • dart/interpret.ts:21 + dart/import-target.ts:47 — Dart's DART_HERITAGE_PREFIX remains a second, un-unified source of the wire prefix. The deliberate-omission rationale (avoid broadening Dart's guard to __property__:) is sound, but the prefix byte now lives in two independent places; a future change to the codec prefix would silently desync Dart's import-target guard — the exact duplicated-literal fragility this PR removes elsewhere. Suggest re-exporting: export const DART_HERITAGE_PREFIX = HERITAGE_MARKER_PREFIX; (keeps heritage-only semantics, single-sources the byte, and gives the exported constant a production consumer).

Nit

  • heritage-marker.ts:17-18HERITAGE_MARKER_PREFIX/PROPERTY_MARKER_PREFIX are consumed only by the test, not production. Wiring Dart to them (above) fixes this; otherwise consider dropping the unused PROPERTY_MARKER_PREFIX export.
  • heritage-marker.ts:2-7 — the shared codec's docstring names "Ruby and Dart"; the code is correctly neutral (no provider branching), but AGENTS.md:42 says shared code "must not name languages." Comment-only; harmless provenance. Optional reword to "the scope resolvers that emit these markers."
  • heritage-marker.test.ts:35-37 — the unit test proves encodeMarker throws on a colon, but the production contract is the inverse (that captures.ts normalization keeps it from throwing during real ingestion). That's currently covered only by code-reading + the existing ruby-qualified-mixin golden; a small include Outer::Mixin end-to-end fixture would lock the unreachability contract directly. Non-blocking.

Strengths

Tight, well-scoped refactor with an honest "no behavioral change" claim that the golden digest actually backs up; the throw-on-colon turns a historical silent edge-drop into a fail-loud; codec is language-neutral in code and sits on the shared path so both registry-primary legs are covered for free. Nice work.

@magyargergo magyargergo force-pushed the fix/1991-ruby-mixin-module-trait branch from ac40fb6 to e94822c Compare June 3, 2026 22:01
@magyargergo magyargergo force-pushed the fix/1994-shared-marker-codec branch from c3922bd to ae0464c Compare June 3, 2026 22:01
magyargergo added a commit that referenced this pull request Jun 3, 2026
…dec (#1994)

Alias DART_HERITAGE_PREFIX to HERITAGE_MARKER_PREFIX (utils/heritage-marker.ts)
instead of re-declaring the '__heritage__:' literal, so the Dart import-target
heritage guard cannot desync from the codec's encode/decode. Value-identical;
gives the codec prefix a direct production consumer. Addresses the tri-review
nit on PR #2007.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@magyargergo

Copy link
Copy Markdown
Collaborator Author

Nit applied + one judgment call (review follow-up)

Single-sourced the Dart wire prefix (f2582ec7). DART_HERITAGE_PREFIX (dart/interpret.ts) is now aliased to the codec's HERITAGE_MARKER_PREFIX (utils/heritage-marker.ts) instead of re-declaring the '__heritage__:' literal — the import-target.ts heritage guard can no longer desync from the codec's encode/decode, and the codec prefix gains a direct production consumer. Value-identical; verified tsc-clean (src) + dart resolver primary 63/63.

Docstring-neutrality nit — left as-is by best judgment. The codec docstring already asserts "Language-NEUTRAL — keyed only on the literal prefixes; no provider branching belongs here", and the Ruby/Dart mention is descriptive (who consumes it), not a logic-level coupling. The AGENTS.md boundary targets language branching in shared logic — the codec has none — so the consumer names are legitimate documentation. Rewording would lose useful "who uses this" context for no boundary benefit.

magyargergo and others added 2 commits June 4, 2026 10:39
…ers (Ruby + Dart) (#1994)

The Ruby and Dart heritage/property pipelines encoded side-effect facts as ':'-delimited synthetic-import marker strings, hand-constructed and hand-parsed at ~8 sites with the field layout kept in agreement only by a comment — the fragility behind the #1981 edge-drop. Route every site through a single shared codec (utils/heritage-marker.ts: encodeMarker / decodeMarker / isHeritageMarker).

encodeMarker throws on a colon-bearing field so the silent-drop class becomes a loud failure; the ':' wire format is preserved byte-for-byte (ruby-captures-golden unchanged). Language-neutral — keyed only on the literal shared prefixes. Dart already single-sources its prefix and is heritage-only, so its import-target guard is left untouched (no invented __property__ path). Pure refactor: no new edges or behavior.

Verified: new codec unit test; ruby resolver + golden 155/155 (zero golden diff) and dart resolver 63/63 on registry-primary, both green on legacy; tsc + prettier clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dec (#1994)

Alias DART_HERITAGE_PREFIX to HERITAGE_MARKER_PREFIX (utils/heritage-marker.ts)
instead of re-declaring the '__heritage__:' literal, so the Dart import-target
heritage guard cannot desync from the codec's encode/decode. Value-identical;
gives the codec prefix a direct production consumer. Addresses the tri-review
nit on PR #2007.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
10594 10578 0 16 581s

✅ All 10578 tests passed

16 test(s) skipped — expand for details
  • COBOL pipeline benchmark > scales with file count
  • C++ ADL emit benchmark > emit phase scales sub-quadratically with co-scaled files and sites
  • C++ pipeline benchmark > scales with file count
  • C# pipeline benchmark > scales with file count — namespaces spread across the solution
  • C# pipeline benchmark > scales with file count — all types in one (global) namespace bucket
  • C# pipeline benchmark > scales with file count — all types in one (named) namespace bucket
  • Go pipeline benchmark > scales with file count (workers enabled)
  • Go pipeline benchmark — worker pool (issue Worker idle timeout kills long Go scope extraction and surfaces as Napi::Error during analyze #1848) > does not quarantine the large generated Go file on sub-batch idle timeout
  • Go structural interface detection benchmark > scales linearly with interface × struct count
  • Go structural interface detection split-phase benchmark > separates index-build and detection time
  • PHP pipeline benchmark > scales with file count (workers enabled)
  • Ruby pipeline benchmark > scales with file count (workers enabled)
  • Rust pipeline benchmark > scales with file count (workers enabled)
  • Vue pipeline benchmark > scales with component count
  • run.cjs direct-exec entrypoint (fix(cli): steer docs, skills, and hooks through a CLI-neutral project-local runner (#1939) #1945) > resolves a .cmd shim via the Windows shell branch, passing args and exit code
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 80.37% 37358/46480 N/A% 🟢 ████████████████░░░░
Branches 69.03% 23743/34393 N/A% 🟢 █████████████░░░░░░░
Functions 85.6% 3918/4577 N/A% 🟢 █████████████████░░░
Lines 84.03% 33626/40013 N/A% 🟢 ████████████████░░░░

📋 View full run · Generated by CI

@magyargergo magyargergo merged commit 2aa78a6 into main Jun 4, 2026
29 checks passed
@magyargergo magyargergo deleted the fix/1994-shared-marker-codec branch June 4, 2026 11:00
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.

refactor(ingestion): share an encode/decode pair for __heritage__/__property__ synthetic-import markers (Ruby + Dart)

1 participant