Skip to content

fix(review): skip bytecode and JAR paths in grounding fetch - #3588

Closed
jimcody1995 wants to merge 1 commit into
JSONbored:mainfrom
jimcody1995:fix/review-grounding-skip-bytecode-jar
Closed

fix(review): skip bytecode and JAR paths in grounding fetch#3588
jimcody1995 wants to merge 1 commit into
JSONbored:mainfrom
jimcody1995:fix/review-grounding-skip-bytecode-jar

Conversation

@jimcody1995

Copy link
Copy Markdown
Contributor

Summary

Scope

  • Conventional Commit title format.
  • Focused — review grounding + unit tests only.
  • Follows CONTRIBUTING.md.
  • Small incremental parity fix; no open issue required.

Validation

  • git diff --check
  • npm run test:ci on Node 22
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • Unit test asserts reads excludes all binary paths before fetch

Safety

  • No secrets, auth, or UI changes.
  • N/A for UI Evidence.

UI Evidence

N/A — backend review grounding only.

Notes

Conflict avoidance: Touches only src/review/review-grounding.ts and test/unit/review-grounding.test.ts. Zero overlap with open PRs (#3580 path-matchers Java gRPC, #3585 enrichment binary-extensions, #3586 hardcoded-URL, #3584/#3582 unused-export, #3577 suggested-change blocks, #3550 manual-review label).

Made with Cursor

Avoid fetching Java .class/.jar and Python .pyc/.pyo as full file text
during review grounding — they carry no review signal and waste tokens.

Co-authored-by: Cursor <cursoragent@cursor.com>
@jimcody1995
jimcody1995 requested a review from JSONbored as a code owner July 5, 2026 18:03
@superagent-security

Copy link
Copy Markdown
Contributor

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

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

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-05 18:05:42 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 80/100 · CI pending · blocked

🛑 Suggested Action - Reject/Close

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR adds four bytecode/archive extensions (.class, .jar, .pyc, .pyo) to the existing SKIP_EXT regex in review-grounding.ts so fetchFullFileContents no longer wastes budget inlining unreadable binary artifacts, mirroring the established pattern for image/lockfile/media extensions already in the list. The regex change is a straightforward, low-risk one-line addition and the accompanying test extends the existing binary-skip test case with the four new paths, asserting fetcher.getFileContent is never called for them — consistent with the pre-existing test structure for the other skipped extensions. The change is correct, narrowly scoped, and the test genuinely exercises the real fetchFullFileContents code path (not a fabricated scenario) since it reuses the same candidates.filter(SKIP_EXT.test(...)) logic already covered by prior cases.

Nits — 4 non-blocking
  • diffFilePriority in review-diff.ts (src/review/review-diff.ts) doesn't route these bytecode extensions to the noise/generated bucket (priority 4) the way lockfiles/dist paths are — worth checking whether that matcher should also treat .class/.jar/.pyc/.pyo as low-priority for diff-budget ordering, since it's a separate list from SKIP_EXT and could drift.
  • The PR notes 'no open issue required' for a 'small incremental parity fix' — per repo convention this still needs a linked eligible issue for external contributor PRs unless explicitly maintainer-authorized; worth confirming that exemption applies here.
  • Consider whether the same four extensions should be added to diffFilePriority's noise regex in src/review/review-diff.ts for consistency, since that's a separate matcher from SKIP_EXT and currently only affects diff-budget ordering, not full-file grounding.
  • If this repo tracks a canonical list of skip-worthy binary/generated extensions in more than one place (SKIP_EXT here vs. any similar list in enrichment/binary-extensions per PR feat(enrichment): recognize more binary formats (audio/video/disk/ML) #3585 mentioned in the description), a shared constant would prevent future drift — flagged as informational only, not blocking this PR.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
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 (no linked issue context).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 115 registered-repo PR(s), 80 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jimcody1995; Gittensor profile; 115 PR(s), 0 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: jimcody1995
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: TypeScript, JavaScript
  • Official Gittensor activity: 115 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (No linked issue detected; Maintainer requires a linked issue). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 5, 2026
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.19%. Comparing base (c78c7c2) to head (340ffcb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3588   +/-   ##
=======================================
  Coverage   93.19%   93.19%           
=======================================
  Files         312      312           
  Lines       31864    31864           
  Branches    11661    11661           
=======================================
  Hits        29695    29695           
  Misses       1517     1517           
  Partials      652      652           
Files with missing lines Coverage Δ
src/review/review-grounding.ts 92.85% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

1 participant