Skip to content

feat(enrichment): categorize the full binary inventory as assets - #3497

Closed
davion-knight wants to merge 1 commit into
JSONbored:mainfrom
davion-knight:feat/categorize-full-binary-inventory
Closed

feat(enrichment): categorize the full binary inventory as assets#3497
davion-knight wants to merge 1 commit into
JSONbored:mainfrom
davion-knight:feat/categorize-full-binary-inventory

Conversation

@davion-knight

Copy link
Copy Markdown
Contributor

Summary

categorizeFile (in review-enrichment/src/analysis-context.ts) assigns each changed file a category used for review scheduling/scope. Its asset check used a narrow hand-listed set of binary extensions — png/jpg/jpeg/gif/webp/pdf/zip/gz/zst — so every other binary the shared inventory already knows (mp4, mov, webm, woff2, ttf, wasm, node, safetensors, gguf, onnx, …) fell through to source, mislabeling a media / font / model-weight change as a source change.

This delegates the check to the shared binary-extension inventory:

  • Consistency anchor: BINARY_EXT_RE (from analyzers/binary-extensions) is the same inventory asset-weight and provenance already use; this replaces the duplicated list with it, matching the feat(enrichment): unify binary extension inventory for asset-weight and provenance #3252 unification (and the same consolidation direction as the recently-merged feat(enrichment): skip all binary + lockfile files in history analyzers #3359).
  • .svg preserved: .svg is text (deliberately not in the binary inventory) but is still an asset for categorization, so it is kept explicitly.
  • Purely additive: only binaries that were previously source become asset; every one of the original 9 extensions is in the inventory, so no file that was asset changes, and no other category is affected. The full existing test suite (1015 tests) passes unchanged, confirming nothing relied on a binary being source.

No linked issue

This is a no-issue PR by design: a self-contained classification-coverage fix + de-duplication touching only review-enrichment/. It broadens the asset category to the shared inventory and removes a hand-maintained list; no new external behavior beyond correctly categorizing more binaries. It mirrors the accepted no-issue precedent for the same kind of enrichment change (e.g. the recently-merged #3329 and #3359).

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 case in review-enrichment/test/analysis-context.test.ts drives createAnalysisContext and asserts .mp4/.woff2/.wasm/.safetensors categorize as asset, .svg stays asset, and .ts stays source. npm run rees:test passes (1015 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 categorization only).
  • No API/OpenAPI/MCP behavior change (N/A).
  • No UI changes (N/A — review-enrichment helper).
  • No visible UI change, so no UI Evidence section is required.
  • No docs/changelog changes needed.

Notes

Analogues followed end-to-end: the existing category tests in analysis-context.test.ts (the .zst asset, long-form docs, and case-insensitive lockfile cases), and the recently-merged consolidation PRs #3329 and #3359. The .svg special-case preserves the pre-existing behavior exactly (svg was already an asset here); only genuinely-binary extensions are newly recognized.

categorizeFile only recognized a narrow hand-listed set of binary extensions
(png/jpg/jpeg/gif/webp/pdf/zip/gz/zst) as the `asset` category, so every other
binary the shared inventory already knows — mp4, mov, webm, woff2, wasm, node,
safetensors, gguf, onnx, and the rest — fell through to `source`, mislabeling
a media/font/model-weight change as a source change.

Delegate the check to the shared binary-extension inventory
(`BINARY_EXT_RE` from analyzers/binary-extensions, the same source asset-weight
and provenance use) instead of a duplicated list, matching the JSONbored#3252
unification. `.svg` is text (deliberately not in the binary inventory) but is
still an asset for categorization, so it is kept explicitly. Purely additive —
only binaries that were previously `source` become `asset`; no other category
changes.
@davion-knight
davion-knight requested a review from JSONbored as a code owner July 5, 2026 12:32
@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 12:31:44 UTC

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

🛑 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.
  • AI reviewers agree on a likely critical defect: PR description: you state this is a no-issue PR, but this repo requires external contributor PRs to close or clearly link an eligible open issue
  • link the maintainer-authorized issue or get maintainer authorization recorded before this can proceed. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
This change correctly replaces the local binary-extension allowlist in categorizeFile with the shared BINARY_EXT_RE while preserving .svg as an asset, and the added test exercises representative media/font/runtime/model binaries plus the unchanged source case. The code path is narrow and consistent with the existing category precedence, so I do not see a reachable classification regression in the diff itself. The PR is still not merge-ready under this repo's contributor policy because it explicitly has no linked eligible issue.

Blockers

  • PR description: you state this is a no-issue PR, but this repo requires external contributor PRs to close or clearly link an eligible open issue; link the maintainer-authorized issue or get maintainer authorization recorded before this can proceed.
Nits — 5 non-blocking
  • review-enrichment/test/analysis-context.test.ts:570: nit: the test name says "full binary inventory" but only samples four inventory entries, so rename it to "categorizes shared binary inventory entries as assets" or parameterize across the exported inventory if that is available.
  • review-enrichment/src/analysis-context.ts:449: nit: the explanatory comment is longer than the branch it documents and repeats the PR description; keep the useful .svg exception note and let the shared import name carry the rest.
  • review-enrichment/test/analysis-context.test.ts:570: add at least one legacy extension such as .png or .zst to this new test, or keep relying on the existing .zst test and rename this one so it does not imply exhaustive coverage.
  • review-enrichment/src/analysis-context.ts:449: consider shortening the comment to: `// .svg is text, so it is not part of BINARY_EXT_RE, but review categorization still treats it as an asset.`
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.

Why this is blocked

  • PR description: you state this is a no-issue PR, but this repo requires external contributor PRs to close or clearly link an eligible open issue; link the maintainer-authorized issue or get maintainer authorization recorded before this can proceed.
  • 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 ❌ 3 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: 36 registered-repo PR(s), 28 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor davion-knight; Gittensor profile; 36 PR(s), 0 issue(s).
Gate result ❌ Blocking Repo-configured hard 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: Rust
  • Official Gittensor activity: 36 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 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; AI reviewers agree on a likely critical defect: PR description: you state this is a no-issue PR, but this repo requires external contributor PRs to close or clearly link an eligible open issue; link the maintainer-authorized issue or get maintainer authorization recorded before this can proceed.). 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
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