Skip to content

fix(review): recognize export const enum and abstract class in impact-symbols - #5858

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
lourincedaging0-commits:fix-impact-symbols-exports
Jul 14, 2026
Merged

fix(review): recognize export const enum and abstract class in impact-symbols#5858
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
lourincedaging0-commits:fix-impact-symbols-exports

Conversation

@lourincedaging0-commits

Copy link
Copy Markdown
Contributor

Summary

EXPORTED_DECLARATION_RE in src/review/impact-symbols.ts — the deterministic changed-symbol extractor feeding the impact-map computation (#2183) — had two regex-variant gaps:

  1. export const enum Direction {} matched the bare (?:const|let|var)\s+([\w$]+) alternative before the enum one, capturing the literal word "enum" as the symbol name instead of Direction.
  2. export abstract class Foo {} matched no alternative at all (there was no abstract handling), so an added/changed abstract base class contributed zero symbols to the impact map.

Fix

  • Add a (?:const\s+)?enum\s+([\w$]+) alternative ahead of const|let|var, so const enum is captured as the enum's real name. enum\s+ requires trailing whitespace, so a real export const enumValue = 1 (identifier merely starting with "enum") still falls through to the const boundary — no over-capture.
  • Add an optional (?:abstract\s+)? prefix to the class alternative, covering export abstract class Foo and export default abstract class Foo (mirroring the existing default-prefix handling).
  • Group order keeps function/class in groups 1/2, so boundaryKindForMatch is unchanged — the reordering only moves groups that all resolve to the "export" kind. Public signatures and the empty/unparseable-patch fail-safe are untouched.

Tests

Extends test/unit/impact-symbols.test.ts with regressions for both fixes — export const enum Direction{ name: "Direction", kind: "export" } (asserting the correct name, not merely that it matches), export abstract class Foo and export default abstract class Foo{ name: "Foo", kind: "class" }, plus a guard that export const enumValue = 1 stays a const boundary. All prior declaration-form tests pass unchanged. 100% patch coverage (statements/branches/functions/lines) on the touched module locally.

No regression to previously-working behavior beyond the two corrected cases.

Closes #5841

…-symbols

`EXPORTED_DECLARATION_RE` in impact-symbols.ts had two regex-variant gaps that
fed wrong input to the deterministic impact-map extraction (JSONbored#2183):

- `export const enum Foo {}` matched the bare `const|let|var` alternative
  first and captured the literal word "enum" as the symbol name instead of
  `Foo`.
- `export abstract class Foo {}` matched no alternative at all, so an
  added/changed abstract base class contributed zero symbols to the impact map.

Add a `(?:const\s+)?enum` alternative ahead of `const|let|var` so `const enum`
is captured as the enum's real name (and a real `export const enumValue`, whose
identifier merely starts with "enum", still falls through to the const
boundary), and allow an optional `abstract` prefix on the class alternative
(covering `export default abstract class` too). Group order keeps function/class
in groups 1/2, so `boundaryKindForMatch` is unchanged — the reordering only
moves groups that all resolve to the "export" kind. Public signatures and the
empty-patch fail-safe are untouched.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.11%. Comparing base (cd97822) to head (2302b38).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5858       +/-   ##
===========================================
+ Coverage   66.87%   95.11%   +28.23%     
===========================================
  Files         587      587               
  Lines       46511    46511               
  Branches    14870    14870               
===========================================
+ Hits        31106    44240    +13134     
+ Misses      12930     1515    -11415     
+ Partials     2475      756     -1719     
Flag Coverage Δ
shard-1 43.23% <66.66%> (-0.54%) ⬇️
shard-2 36.29% <33.33%> (?)
shard-3 32.02% <33.33%> (-0.11%) ⬇️
shard-4 31.89% <100.00%> (-0.92%) ⬇️
shard-5 32.49% <33.33%> (?)
shard-6 44.86% <33.33%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/review/impact-symbols.ts 100.00% <100.00%> (ø)

... and 304 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 14, 2026
@loopover-orb

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-14 20:16:00 UTC

2 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This narrowly fixes two real bugs in `EXPORTED_DECLARATION_RE` (src/review/impact-symbols.ts:34-51): `export const enum Foo` previously captured the literal word "enum" as the name because the bare `const|let|var` alternative matched first, and `export abstract class Foo` matched no alternative at all and silently contributed zero symbols. The new regex reorders `(?:const\s+)?enum\s+` ahead of `const|let|var` and adds an optional `(?:abstract\s+)?` prefix to the class alternative, and I traced the alternation order/backtracking manually — both fixes are correct and `export const enumValue = 1` still falls through to the const boundary as intended since `enum\s+` requires trailing whitespace. `boundaryKindForMatch` is unaffected since function/class stay in groups 1/2 and every other alternative still maps to "export". Tests exercise both new cases plus the no-over-capture guard and the default-abstract-class combination, giving solid branch coverage on the touched lines.

Nits — 2 non-blocking
  • The module-doc comment block above the regex (src/review/impact-symbols.ts:37-45) is quite long for a one-line regex change — could be trimmed to the two sentences that actually explain the ordering rationale.
  • Consider a quick grep for other regexes in the codebase (e.g. rag.ts's BOUNDARY_RE) that share the same const/enum ordering issue, since this bug pattern could recur there too.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5841
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 33 registered-repo PR(s), 12 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor lourincedaging0-commits; Gittensor profile; 33 PR(s), 0 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: minor — It's a small, well-tested, correctly-traced regex fix closing a linked issue, not a structural or high-leverage change to the review engine.
Linked issue satisfaction

Addressed
The diff reorders EXPORTED_DECLARATION_RE so `(?:const\s+)?enum` is tried before the bare const/let/var alternative (fixing the mis-captured "enum" name) and adds an optional `abstract\s+` prefix to the class alternative (fixing the dropped abstract-class case), while boundaryKindForMatch remains correct since function/class stay in groups 1/2. New regression tests cover const enum, abstract class

Review context
  • Author: lourincedaging0-commits
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 33 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit da3e9ad into JSONbored:main Jul 14, 2026
15 checks passed
This was referenced Jul 14, 2026
This was referenced Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(review): impact-symbols mis-parses export const enum and drops export abstract class

1 participant