Skip to content

feat(enrichment): skip all binary + lockfile files in history analyzers - #3359

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
davion-knight:feat/history-analyzers-unified-skip
Jul 5, 2026
Merged

feat(enrichment): skip all binary + lockfile files in history analyzers#3359
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
davion-knight:feat/history-analyzers-unified-skip

Conversation

@davion-knight

Copy link
Copy Markdown
Contributor

Summary

The two history-class analyzers — blame-link (#2034) and churn-hotspot (#1513) — each carried an identical, hand-maintained SKIP_RE that skips files whose commit history is not a useful "who-introduced-this" / fragility signal (lockfiles, generated output, binaries). That regex only enumerated a narrow binary subset (png/jpe?g/gif/svg/ico/pdf/zip/gz/woff2?) and the five npm/yarn/pnpm/poetry/go lockfiles. So any binary it didn't list — webp, avif, heic, mp4, wasm, safetensors, and the rest of the shared inventory — plus Rust/PHP/Bun lockfiles were still treated as candidate history signals, producing noise on files that carry no code signal.

This change:

  • Extracts the rule into a shared isHistoryUninformativePath() (new analyzers/history-path.ts). Both analyzers import it and drop their local copy, so the two can no longer drift apart (the duplicated regex was a latent desync hazard).
  • Broadens coverage by delegating binary and lockfile recognition to the unified inventoriesbinary-extensions' BINARY_EXT_RE and lockfile-path's isSupportedLockfile — instead of a hand-maintained list, so it stays in sync as those grow. This follows the same consolidation direction as feat(enrichment): unify binary extension inventory for asset-weight and provenance #3252 (unified binary-extension inventory).
  • Purely additive: the original regex is kept verbatim inside the shared helper and the two inventory checks are OR-ed on top, so it only ever skips more files, never fewer. No file that was analyzed before is newly skipped except genuine binaries/lockfiles that should have been skipped all along.

No linked issue

This is a no-issue PR by design: a self-contained analyzer refactor + coverage fix touching only review-enrichment/. It removes a duplicated skip regex and broadens binary/lockfile coverage using existing shared helpers; no new external behavior beyond skipping more non-code files. It mirrors the accepted no-issue precedent for the same kind of enrichment change (e.g. the recently-merged #3264 and #3329).

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • No linked issue — see the No linked issue section above.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage — no src/** lines changed (this change is under review-enrichment/, which Codecov does not measure), so codecov/patch has no diff to gate; suite is green.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New behavior has tests — a new review-enrichment/test/history-path.test.ts unit-covers isHistoryUninformativePath across every branch (original lockfile/generated/dir set, the broader shared binary inventory, the full lockfile inventory, and negatives like Cargo.toml/cargo.lock.md/source files); the existing churn-hotspot skip test is extended to prove a .safetensors blob and a Cargo.lock are now skipped without a fetch. npm run rees:test passes (929 tests; analyzer-metadata check clean).

Validated green against the full GitHub CI validate-code check set — actionlint, db:migrations:check, db:schema-drift:check, cf-typegen:check, selfhost:validate-observability, typecheck, test:coverage, test:workers, build:mcp, test:mcp-pack, build:miner, rees:test, ui:openapi:check, ui:openapi:settings-parity, ui:version-audit, ui:lint, ui:typecheck, ui:test, ui:build. Branch rebased on latest main.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • No auth, cookie, CORS, GitHub App, Cloudflare, or session changes (N/A — internal analyzer skip logic only).
  • No API/OpenAPI/MCP behavior change (N/A).
  • No UI changes (N/A — review-enrichment analyzers).
  • No visible UI change, so no UI Evidence section is required.
  • No docs/changelog changes needed.

Notes

Both history analyzers are advisory, metadata-only (they surface a PR number / short SHA / counts from the public commit log, never file contents), and this change does not alter that — it only widens the set of non-code files they decline to spend a GitHub round-trip on. The original SKIP_RE is preserved verbatim inside the shared helper so its .min.js/.map/.snap/.svg/dist|build|vendor matches are unchanged; only binary and lockfile recognition is broadened via the shared inventories.

blame-link (JSONbored#2034) and churn-hotspot (JSONbored#1513) each carried an identical
hand-maintained SKIP_RE listing a narrow subset of binaries (png/jpg/gif/
svg/ico/pdf/zip/gz/woff) and lockfiles. Any binary the list did not enumerate
— webp, avif, heic, mp4, wasm, safetensors, and the rest of the shared
inventory — plus Rust/PHP/Bun lockfiles were still treated as candidate
"who-introduced-this" / churn-fragility signals, producing noise on files
whose commit history carries no code signal.

Extract the rule into a shared isHistoryUninformativePath() (new
analyzers/history-path.ts) that keeps the original regex verbatim and
additionally delegates binary and lockfile recognition to the unified
inventories (binary-extensions' BINARY_EXT_RE, lockfile-path's
isSupportedLockfile). Both analyzers now import it, removing the duplicated
regex so the two can no longer drift apart. The change is purely additive —
it only ever skips more, never fewer, files.
@davion-knight
davion-knight requested a review from JSONbored as a code owner July 5, 2026 04:45
@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

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-05 04:46:02 UTC

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

⏸️ Suggested Action - Manual Review

Review summary
This refactor centralizes the duplicated history-skip predicate and wires both history analyzers through the same helper while preserving the old regex and adding shared binary/lockfile inventories. The reachable analyzer paths still skip added/generated/binary/lockfile files before GitHub calls, and the new helper has direct unit coverage plus churn integration coverage. I do not see a correctness blocker in the provided diff.

Nits — 5 non-blocking
  • nit: review-enrichment/test/history-path.test.ts:37 uses `vendor/lib.wasm`, which is skipped by the preserved `vendor/` directory rule even if `BINARY_EXT_RE` did not include `wasm`, so that case is weaker than the comment claims.
  • nit: review-enrichment/test/churn-hotspot.test.ts:74 proves the shared predicate is used by churn-hotspot, but the corresponding changed blame-link filter line has no visible integration assertion for the newly broadened skips.
  • review-enrichment/test/history-path.test.ts:37: change `vendor/lib.wasm` to a neutral path such as `pkg/lib.wasm` so the test actually proves shared binary-extension coverage.
  • review-enrichment/test/blame-link.test.ts: add the same no-fetch assertion for `models/llama.safetensors` and `crates/api/Cargo.lock`, or tell me why helper-unit coverage alone is the intended standard for this analyzer wiring.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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: 33 registered-repo PR(s), 24 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor davion-knight; Gittensor profile; 33 PR(s), 0 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: davion-knight
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 33 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Explain no-issue PR.
  • 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 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 4973f5c into JSONbored:main Jul 5, 2026
6 checks passed
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